(core) Only update changed records with RenameChoices

Summary:
Make _rename_cell_choice return None for unchanged values

The tests actually passs without the implementation changes, because trim_update_action removed the noop updates. So I'm not sure if this is an improvement. It certainly seems that it would be slower in cases where every record is updated, and it's hard to say if it would be better in other cases.

Test Plan:
Checked doc actions in existing test.
Also tested for invalid existing values.

Reviewers: dsagal

Reviewed By: dsagal

Subscribers: jarek

Differential Revision: https://phab.getgrist.com/D3052

sandbox/grist/column.py

@@ -225,6 +225,15 @@ class BaseColumn(object):
     # pylint: disable=no-self-use, unused-argument
     return values, []
 
+
+class DataColumn(BaseColumn):
+  """
+  DataColumn describes a column of raw data, and holds it.
+  """
+  pass
+
+
+class ChoiceColumn(DataColumn):
   def rename_choices(self, renames):
     row_ids = []
     values = []
@@ -237,15 +246,9 @@ class BaseColumn(object):
     return row_ids, values
 
   def _rename_cell_choice(self, renames, value):
-    return renames.get(value, value)
+    return renames.get(value)
 
 
-class DataColumn(BaseColumn):
-  """
-  DataColumn describes a column of raw data, and holds it.
-  """
-  pass
-
 class BoolColumn(BaseColumn):
   def set(self, row_id, value):
     # When 1 or 1.0 is loaded, we should see it as True, and similarly 0 as False. This is similar
@@ -356,7 +359,7 @@ class PositionColumn(NumericColumn):
     return new_values, [(self._sorted_rows[i], pos) for (i, pos) in adjustments]
 
 
-class ChoiceListColumn(BaseColumn):
+class ChoiceListColumn(ChoiceColumn):
   """
   ChoiceListColumn's default value is None, but is presented to formulas as the empty list.
   """
@@ -376,7 +379,8 @@ class ChoiceListColumn(BaseColumn):
     return () if typed_value is None else typed_value
 
   def _rename_cell_choice(self, renames, value):
-    return tuple(renames.get(choice, choice) for choice in value)
+    if any((v in renames) for v in value):
+      return tuple(renames.get(choice, choice) for choice in value)
 
 
 class BaseReferenceColumn(BaseColumn):
@@ -502,6 +506,7 @@ class ReferenceListColumn(BaseReferenceColumn):
 usertypes.BaseColumnType.ColType = DataColumn
 usertypes.Reference.ColType = ReferenceColumn
 usertypes.ReferenceList.ColType = ReferenceListColumn
+usertypes.Choice.ColType = ChoiceColumn
 usertypes.ChoiceList.ColType = ChoiceListColumn
 usertypes.DateTime.ColType = DateTimeColumn
 usertypes.Date.ColType = DateColumn

sandbox/grist/test_useractions.py

@@ -791,6 +791,8 @@ class TestUserActions(test_engine.EngineTestCase):
           [3, "c"],
           [4, "d"],
           [5, None],
+          [6, 5],
+          [7, [[]]],
         ],
         "ChoiceListTable": [
           ["id", "ChoiceListColumn"],
@@ -803,6 +805,8 @@ class TestUserActions(test_engine.EngineTestCase):
           [8, ["b", "c"]],
           [9, ["a", "c"]],
           [10, ["a", "b", "c"]],
+          [11, 5],
+          [12, [[]]],
         ],
       }
     })
@@ -811,11 +815,33 @@ class TestUserActions(test_engine.EngineTestCase):
     # Renames go in a loop to make sure that works correctly
     # a -> b -> c -> a -> b -> ...
     renames = {"a": "b", "b": "c", "c": "a"}
-    self.apply_user_action(
+    out_actions_choice = self.apply_user_action(
       ["RenameChoices", "ChoiceTable", "ChoiceColumn", renames])
-    self.apply_user_action(
+    out_actions_choice_list = self.apply_user_action(
       ["RenameChoices", "ChoiceListTable", "ChoiceListColumn", renames])
 
+    self.assertPartialOutActions(
+      out_actions_choice,
+      {'stored':
+         [['BulkUpdateRecord',
+           'ChoiceTable',
+           [1, 2, 3],
+           {'ChoiceColumn': [u'b', u'c', u'a']}]]})
+
+    self.assertPartialOutActions(
+      out_actions_choice_list,
+      {'stored':
+         [['BulkUpdateRecord',
+           'ChoiceListTable',
+           [1, 2, 3, 7, 8, 9, 10],
+           {'ChoiceListColumn': [['L', u'b'],
+                                 ['L', u'c'],
+                                 ['L', u'a'],
+                                 ['L', u'b', u'c'],
+                                 ['L', u'c', u'a'],
+                                 ['L', u'b', u'a'],
+                                 ['L', u'b', u'c', u'a']]}]]})
+
     self.assertTableData('ChoiceTable', data=[
       ["id", "ChoiceColumn"],
       [1, "b"],
@@ -823,6 +849,8 @@ class TestUserActions(test_engine.EngineTestCase):
       [3, "a"],
       [4, "d"],
       [5, None],
+      [6, 5],
+      [7, [[]]],
     ])
 
     self.assertTableData('ChoiceListTable', data=[
@@ -836,4 +864,6 @@ class TestUserActions(test_engine.EngineTestCase):
       [8, ["c", "a"]],
       [9, ["b", "a"]],
       [10, ["b", "c", "a"]],
+      [11, 5],
+      [12, [[]]],
     ])