(core) Adding backend for 2-way references

Summary:
Adding support for 2-way references in data engine.

- Columns have an `reverseCol` field, which says "this is a reverse of the given column, update me when that one changes".
- At the time of setting `reverseCol`, we ensure that it's symmetrical to make a 2-way reference.
- Elsewhere we just implement syncing in one direction:
  - When `reverseCol` is present, user code is generated with a type like `grist.ReferenceList("Tasks", reverse_of="Assignee")`
  - On updating a ref column, we use `prepare_new_values()` method to generate corresponding updates to any column that's a reverse of it.
  - The `prepare_new_values()` approach is extended to support this.
  - We don't add (or remove) any mappings between rows, and rely on existing mappings (in a ref column's `_relation`) to create reverse updates.

NOTE This is polished version of https://phab.getgrist.com/D4307 with tests and 3 bug fixes
- Column transformation didn't work when transforming RefList to Ref, the reverse column became out of sync
- Tables with reverse columns couldn't be removed
- Setting json arrays to RefList didn't work if arrays contained other things besides ints
Those fixes are covered by new tests.

Test Plan: New tests

Reviewers: georgegevoian, paulfitz, dsagal

Reviewed By: georgegevoian, paulfitz

Subscribers: dsagal

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

sandbox/grist/column.py

@@ -6,11 +6,13 @@ from numbers import Number
 
 import six
 
+import actions
 import depend
 import objtypes
 import usertypes
 import relabeling
 import relation
+import reverse_references
 import moment
 from sortedcontainers import SortedListWithKey
 
@@ -221,17 +223,23 @@ class BaseColumn(object):
     """
     return self.type_obj.convert(value_to_convert)
 
-  def prepare_new_values(self, values, ignore_data=False, action_summary=None):
+  def prepare_new_values(self, row_ids, values, ignore_data=False, action_summary=None):
     """
-    This allows us to modify values and also produce adjustments to existing records. This
-    currently is only used by PositionColumn. Returns two lists: new_values, and
-    [(row_id, new_value)] list of adjustments to existing records.
+    This allows us to modify values and also produce adjustments to existing records.
+
+    Returns the pair (new_values, adjustments), where new_values is a list to replace `values`
+    (one for each row_id), and adjustments is a list of additional docactions to apply, e.g. to
+    adjust other rows.
+
     If ignore_data is True, makes adjustments without regard to the existing data; this is used
     for processing ReplaceTableData actions.
     """
     # pylint: disable=no-self-use, unused-argument
     return values, []
 
+  def recalc_from_reverse_values(self):
+    pass    # Only two-way references implement this
+
 
 class DataColumn(BaseColumn):
   """
@@ -253,6 +261,7 @@ class ChoiceColumn(DataColumn):
     return row_ids, values
 
   def _rename_cell_choice(self, renames, value):
+    # pylint: disable=no-self-use
     return renames.get(value)
 
 
@@ -373,7 +382,7 @@ class PositionColumn(NumericColumn):
     self._sorted_rows = SortedListWithKey(other_column._sorted_rows[:],
                                           key=lambda x: SafeSortKey(self.raw_get(x)))
 
-  def prepare_new_values(self, values, ignore_data=False, action_summary=None):
+  def prepare_new_values(self, row_ids, values, ignore_data=False, action_summary=None):
     # This does the work of adjusting positions and relabeling existing rows with new position
     # (without changing sort order) to make space for the new positions. Note that this is also
     # used for updating a position for an existing row: we'll find a new value for it; later when
@@ -385,7 +394,9 @@ class PositionColumn(NumericColumn):
     # prepare_inserts expects floats as keys, not MixedTypesKeys
     rows = SortedListWithKey(rows, key=self.raw_get)
     adjustments, new_values = relabeling.prepare_inserts(rows, values)
-    return new_values, [(self._sorted_rows[i], pos) for (i, pos) in adjustments]
+    adj_action = _adjustments_to_action(self.node,
+        [(self._sorted_rows[i], pos) for (i, pos) in adjustments])
+    return new_values, ([adj_action] if adj_action else [])
 
 
 class ChoiceListColumn(ChoiceColumn):
@@ -410,6 +421,7 @@ class ChoiceListColumn(ChoiceColumn):
   def _rename_cell_choice(self, renames, value):
     if any((v in renames) for v in value):
       return tuple(renames.get(choice, choice) for choice in value)
+    return None
 
 
 class BaseReferenceColumn(BaseColumn):
@@ -420,24 +432,45 @@ class BaseReferenceColumn(BaseColumn):
     super(BaseReferenceColumn, self).__init__(table, col_id, col_info)
     # We can assume that all tables have been instantiated, but not all initialized.
     target_table_id = self.type_obj.table_id
+    self._table = table
     self._target_table = table._engine.tables.get(target_table_id, None)
     self._relation = relation.ReferenceRelation(table.table_id, target_table_id, col_id)
     # Note that we need to remove these back-references when the column is removed.
     if self._target_table:
       self._target_table._back_references.add(self)
 
+    self._reverse_source_node = self.type_obj.reverse_source_node()
+    if self._reverse_source_node:
+      _multimap_add(self._table._reverse_cols_by_source_node, self._reverse_source_node, self)
+
+
   def destroy(self):
     # Destroy the column and remove the back-reference we created in the constructor.
     super(BaseReferenceColumn, self).destroy()
+    if self._reverse_source_node:
+      _multimap_remove(self._table._reverse_cols_by_source_node, self._reverse_source_node, self)
+
     if self._target_table:
       self._target_table._back_references.remove(self)
 
   def _update_references(self, row_id, old_value, new_value):
+    for r in self._value_iterable(old_value):
+      self._relation.remove_reference(row_id, r)
+    for r in self._value_iterable(new_value):
+      self._relation.add_reference(row_id, r)
+
+  def _clean_up_value(self, value):
+    raise NotImplementedError()
+
+  def _value_iterable(self, value):
+    raise NotImplementedError()
+
+  def _list_to_value(self, value_as_list):
     raise NotImplementedError()
 
   def set(self, row_id, value):
     old = self.safe_get(row_id)
-    super(BaseReferenceColumn, self).set(row_id, value)
+    super(BaseReferenceColumn, self).set(row_id, self._clean_up_value(value))
     new = self.safe_get(row_id)
     self._update_references(row_id, old, new)
 
@@ -462,6 +495,39 @@ class BaseReferenceColumn(BaseColumn):
     affected_rows = sorted(self._relation.get_affected_rows(target_row_ids))
     return [(row_id, self._raw_get_without(row_id, target_row_ids)) for row_id in affected_rows]
 
+  def prepare_new_values(self, row_ids, values, ignore_data=False, action_summary=None):
+    values = [self._clean_up_value(v) for v in values]
+    reverse_cols = self._target_table._reverse_cols_by_source_node.get(self.node, [])
+    adjustments = []
+    if reverse_cols:
+      old_values = [self.raw_get(r) for r in row_ids]
+      reverse_adjustments = reverse_references.get_reverse_adjustments(
+          row_ids, old_values, values, self._value_iterable, self._relation)
+
+      if reverse_adjustments:
+        for reverse_col in reverse_cols:
+          adjustments.append(_adjustments_to_action(
+            reverse_col.node,
+            [(row_id, reverse_col._list_to_value(value)) for (row_id, value) in reverse_adjustments]
+          ))
+
+    return values, adjustments
+
+  def recalc_from_reverse_values(self):
+    """
+    Generates actions to update reverse column based on this column.
+    """
+    if not self._reverse_source_node:
+      return None
+    rev_table_id, rev_col_id = self._reverse_source_node
+    reverse_col = self._target_table.get_column(rev_col_id)
+    reverse_adjustments = []
+    for target_row_id in self._target_table.row_ids:
+      reverse_value = self._relation.get_affected_rows((target_row_id,))
+      reverse_adjustments.append((target_row_id, sorted(reverse_value)))
+    return _adjustments_to_action(reverse_col.node,
+        [(row_id, reverse_col._list_to_value(value)) for (row_id, value) in reverse_adjustments])
+
   def _raw_get_without(self, _row_id, _target_row_ids):
     """
     Returns a Ref or RefList cell value with the specified target_row_ids removed, assuming one of
@@ -493,28 +559,33 @@ class ReferenceColumn(BaseReferenceColumn):
     # the 0 index will contain the all-defaults record.
     return self._target_table.Record(typed_value, self._relation)
 
-  def _update_references(self, row_id, old_value, new_value):
-    if old_value:
-      self._relation.remove_reference(row_id, old_value)
-    if new_value:
-      self._relation.add_reference(row_id, new_value)
+  def _value_iterable(self, value):
+    return (value,) if value and self.type_obj.is_right_type(value) else ()
 
-  def set(self, row_id, value):
+  def _list_to_value(self, value_as_list):
+    if len(value_as_list) > 1:
+      raise ValueError("UNIQUE reference constraint failed for action")
+    return value_as_list[0] if value_as_list else 0
+
+  def _clean_up_value(self, value):
     # Allow float values that are small integers. In practice, this only turns out to be relevant
     # in rare cases (such as undo of Ref->Numeric conversion).
     if type(value) == float and value.is_integer():   # pylint:disable=unidiomatic-typecheck
       if value > 0 and objtypes.is_int_short(int(value)):
-        value = int(value)
-    super(ReferenceColumn, self).set(row_id, value)
+        return int(value)
+    return value
 
-  def prepare_new_values(self, values, ignore_data=False, action_summary=None):
+  def prepare_new_values(self, row_ids, values, ignore_data=False, action_summary=None):
     if action_summary and values:
       values = action_summary.translate_new_row_ids(self._target_table.table_id, values)
-    return values, []
+    return super(ReferenceColumn, self).prepare_new_values(row_ids, values,
+        ignore_data=ignore_data, action_summary=action_summary)
 
   def convert(self, val):
     if isinstance(val, objtypes.ReferenceLookup):
       val = self._lookup(val, val.value) or self._alt_text(val.alt_text)
+    elif isinstance(val, list):
+      val = val[0] if val else 0
     return super(ReferenceColumn, self).convert(val)
 
 
@@ -523,7 +594,7 @@ class ReferenceListColumn(BaseReferenceColumn):
   ReferenceListColumn maintains for each row a list of references (row IDs) into another table.
   Accessing them yields RecordSets.
   """
-  def set(self, row_id, value):
+  def _clean_up_value(self, value):
     if isinstance(value, six.string_types):
       # This is second part of a "hack" we have to do when we rename tables. During
       # the rename, we briefly change all Ref columns to Int columns (to lose the table
@@ -535,20 +606,27 @@ class ReferenceListColumn(BaseReferenceColumn):
       try:
         # If it's a string that looks like JSON, try to parse it as such.
         if value.startswith('['):
-          value = json.loads(value)
+          parsed = json.loads(value)
+
+          # It must be list of integers.
+          if not isinstance(parsed, list):
+            return value
+
+          # All of them must be positive integers
+          if all(isinstance(v, int) and v > 0 for v in parsed):
+            return parsed
         else:
-        # Else try to parse it as a RecordList
-          value = objtypes.RecordList.from_repr(value)
+          # Else try to parse it as a RecordList
+          return objtypes.RecordList.from_repr(value)
       except Exception:
         pass
+    return value
 
-    super(ReferenceListColumn, self).set(row_id, value)
+  def _value_iterable(self, value):
+    return value if value and self.type_obj.is_right_type(value) else ()
 
-  def _update_references(self, row_id, old_list, new_list):
-    for old_value in old_list or ():
-      self._relation.remove_reference(row_id, old_value)
-    for new_value in new_list or ():
-      self._relation.add_reference(row_id, new_value)
+  def _list_to_value(self, value_as_list):
+    return value_as_list or None
 
   def _make_rich_value(self, typed_value):
     if typed_value is None:
@@ -579,8 +657,27 @@ class ReferenceListColumn(BaseReferenceColumn):
           return self._alt_text(val.alt_text)
         result.append(lookup_value)
       val = result
+
+    if isinstance(val, int) and val:
+      val = [val]
+
     return super(ReferenceListColumn, self).convert(val)
 
+def _multimap_add(mapping, key, value):
+  mapping.setdefault(key, []).append(value)
+
+def _multimap_remove(mapping, key, value):
+  if key in mapping and value in mapping[key]:
+    mapping[key].remove(value)
+    if not mapping[key]:
+      del mapping[key]
+
+def _adjustments_to_action(node, row_value_pairs):
+  # Takes a depend.Node and a list of (row_id, value) pairs, and returns a BulkUpdateRecord action.
+  if not row_value_pairs:
+    return None
+  row_ids, values = zip(*row_value_pairs)
+  return actions.BulkUpdateRecord(node.table_id, row_ids, {node.col_id: values})
 
 # Set up the relationship between usertypes objects and column objects.
 usertypes.BaseColumnType.ColType = DataColumn

sandbox/grist/docactions.py

@@ -166,8 +166,7 @@ class DocActions(object):
     # Replace the renamed column in the schema object.
     schema_table_info = self._engine.schema[table_id]
     colinfo = schema_table_info.columns.pop(old_col_id)
-    schema_table_info.columns[new_col_id] = schema.SchemaColumn(
-      new_col_id, colinfo.type, colinfo.isFormula, colinfo.formula)
+    schema_table_info.columns[new_col_id] = colinfo._replace(colId=new_col_id)
 
     self._engine.rebuild_usercode()
     self._engine.new_column_name(table)
@@ -192,12 +191,14 @@ class DocActions(object):
     new = schema.SchemaColumn(col_id,
                               col_info.get('type', old.type),
                               bool(col_info.get('isFormula', old.isFormula)),
-                              col_info.get('formula', old.formula))
+                              col_info.get('formula', old.formula),
+                              col_info.get('reverseColId', old.reverseColId))
     if new == old:
       log.info("ModifyColumn called which was a noop")
       return
 
-    undo_col_info = {k: v for k, v in six.iteritems(schema.col_to_dict(old, include_id=False))
+    undo_col_info = {k: v for k, v in six.iteritems(
+                        schema.col_to_dict(old, include_id=False, include_default=True))
                      if k in col_info}
 
     # Remove the column from the schema, then re-add it, to force creation of a new column object.

sandbox/grist/engine.py

@@ -507,9 +507,11 @@ class Engine(object):
     if col_parent_ids > valid_table_refs:
       collist = sorted(actions.transpose_bulk_action(meta_columns),
                        key=lambda c: (c.parentId, c.parentPos))
+      reverse_col_id = schema.get_reverse_col_id_lookup_func(collist)
       raise AssertionError("Internal schema inconsistent; extra columns in metadata:\n"
           + "\n".join('  #%s %s' %
-                      (c.id, schema.SchemaColumn(c.colId, c.type, bool(c.isFormula), c.formula))
+                      (c.id, schema.SchemaColumn(c.colId, c.type, bool(c.isFormula), c.formula,
+                        reverse_col_id(c)))
                       for c in collist if c.parentId not in valid_table_refs))
 
   def dump_state(self):
@@ -1035,11 +1037,9 @@ class Engine(object):
 
       # If there are values for any PositionNumber columns, ensure PositionNumbers are ordered as
       # intended but are all unique, which may require updating other positions.
-      nvalues, adjustments = col_obj.prepare_new_values(values,
+      nvalues, adjustments = col_obj.prepare_new_values(row_ids, values,
           action_summary=self.out_actions.summary)
-      if adjustments:
-        extra_actions.append(actions.BulkUpdateRecord(
-          action.table_id, [r for r,v in adjustments], {col_id: [v for r,v in adjustments]}))
+      extra_actions.extend(adjustments)
 
       new_values[col_id] = nvalues
 
@@ -1054,11 +1054,10 @@ class Engine(object):
         defaults = [col_obj.getdefault() for r in row_ids]
         # We use defaults to get new values or adjustments. If we are replacing data, we'll make
         # the adjustments without regard to the existing data.
-        nvalues, adjustments = col_obj.prepare_new_values(defaults, ignore_data=ignore_data,
+        nvalues, adjustments = col_obj.prepare_new_values(row_ids, defaults,
+            ignore_data=ignore_data,
             action_summary=self.out_actions.summary)
-        if adjustments:
-          extra_actions.append(actions.BulkUpdateRecord(
-            action.table_id, [r for r,v in adjustments], {col_id: [v for r,v in adjustments]}))
+        extra_actions.extend(adjustments)
         if nvalues != defaults:
           new_values[col_id] = nvalues
 

sandbox/grist/gencode.py

@@ -42,7 +42,7 @@ def indent(body, levels=1):
 
 #----------------------------------------------------------------------
 
-def get_grist_type(col_type):
+def get_grist_type(col_type, reverse_col_id=None):
   """Returns code for a grist usertype object given a column type string."""
   col_type_split = col_type.split(':', 1)
   typename = col_type_split[0]
@@ -54,7 +54,12 @@ def get_grist_type(col_type):
   arg = col_type_split[1] if len(col_type_split) > 1 else ''
   arg = arg.strip().replace("'", "\\'")
 
-  return "grist.%s(%s)" % (typename, ("'%s'" % arg) if arg else '')
+  args = []
+  if arg:
+    args.append("'%s'" % arg)
+  if reverse_col_id and typename in ('Reference', 'ReferenceList'):
+    args.append('reverse_of=' + repr(reverse_col_id))
+  return "grist.%s(%s)" % (typename, ", ".join(args))
 
 
 class GenCode(object):
@@ -99,7 +104,7 @@ class GenCode(object):
 
     decorator = ''
     if include_type and col_info.type != 'Any':
-      decorator = '@grist.formulaType(%s)\n' % get_grist_type(col_info.type)
+      decorator = '@grist.formulaType(%s)\n' % get_grist_type(col_info.type, col_info.reverseColId)
     return textbuilder.Combiner(['\n' + decorator + decl, indent(body), '\n'])
 
 
@@ -111,7 +116,8 @@ class GenCode(object):
                                             name=table.get_default_func_name(col_info.colId),
                                             include_type=False,
                                             additional_params=['value', 'user']))
-    parts.append("%s = %s\n" % (col_info.colId, get_grist_type(col_info.type)))
+    parts.append("%s = %s\n" % (col_info.colId,
+      get_grist_type(col_info.type, col_info.reverseColId)))
     return textbuilder.Combiner(parts)
 
 

sandbox/grist/migrations.py

@@ -1317,3 +1317,11 @@ def migration42(tdset):
     add_column('_grist_Triggers', 'watchedColRefList', 'RefList:_grist_Tables_column'),
     add_column('_grist_Triggers', 'options', 'Text'),
   ])
+
+@migration(schema_version=43)
+def migration43(tdset):
+  """
+  Adds reverseCol for two-way references.
+  """
+  return tdset.apply_doc_actions([
+    add_column('_grist_Tables_column', 'reverseCol', 'Ref:_grist_Tables_column')])

sandbox/grist/records.py

@@ -111,6 +111,11 @@ class Record(object):
   def __repr__(self):
     return "%s[%s]" % (self._table.table_id, self._row_id)
 
+  def _exists(self):
+    # Whether the record exists: helpful for the rare cases when examining a record with a
+    # non-zero rowId which has just been deleted.
+    return self._row_id in self._table.row_ids
+
   def _clone_with_relation(self, src_relation):
     return self._table.Record(self._row_id,
                               relation=src_relation.compose(self._source_relation))

sandbox/grist/reverse_references.py

@@ -0,0 +1,73 @@
+from collections import defaultdict
+import six
+from usertypes import get_referenced_table_id
+
+class _RefUpdates(object):
+  def __init__(self):
+    self.removals = set()
+    self.additions = set()
+
+def get_reverse_adjustments(row_ids, old_values, new_values, value_iterator, relation):
+  """
+  Generates data for updating reverse columns, based on changes to this column
+  """
+
+  # Stores removals and addons for each target row
+  affected_target_rows = defaultdict(_RefUpdates)
+
+  # Iterate over changes to source column (my column)
+  for (source_row_id, old_value, new_value) in zip(row_ids, old_values, new_values):
+    if new_value != old_value:
+      # Treat old_values as removals, and new_values as additions
+      for target_row_id in value_iterator(old_value):
+        affected_target_rows[target_row_id].removals.add(source_row_id)
+      for target_row_id in value_iterator(new_value):
+        affected_target_rows[target_row_id].additions.add(source_row_id)
+
+  # Now in affected_target_rows, we have the changes (deltas), now we are going to convert them
+  # to updates (full list of values) in target columns.
+
+  adjustments = []
+
+  # For each target row (that needs to be updated, and was change in our column)
+  for target_row_id, updates in six.iteritems(affected_target_rows):
+    # Get the value stored in that column by using our own relation object (which should store
+    # correct values - the same that are stored in that reverse column). `reverse_value` is the
+    # value in that reverse cell
+    reverse_value = relation.get_affected_rows((target_row_id,))
+
+    # Now make the adjustments using calculated deltas
+    for source_row_id in updates.removals:
+      reverse_value.discard(source_row_id)
+    for source_row_id in updates.additions:
+      reverse_value.add(source_row_id)
+    adjustments.append((target_row_id, sorted(reverse_value)))
+
+  return adjustments
+
+
+def check_desired_reverse_col(col_type, desired_reverse_col):
+  if not desired_reverse_col:
+    raise ValueError("invalid column specified in reverseCol")
+  if desired_reverse_col.reverseCol:
+    raise ValueError("reverseCol specifies an existing two-way reference column")
+  ref_table_id = get_referenced_table_id(col_type)
+  if not ref_table_id:
+    raise ValueError("reverseCol may only be set on a column with a reference type")
+  if desired_reverse_col.tableId != ref_table_id:
+    raise ValueError("reverseCol must be a column in the target table")
+
+
+def pick_reverse_col_label(docmodel, col_rec):
+  ref_table_id = get_referenced_table_id(col_rec.type)
+  ref_table_rec = docmodel.get_table_rec(ref_table_id)
+
+  # First try the source table title.
+  source_table_rec = col_rec.parentId
+  reverse_label = source_table_rec.rawViewSectionRef.title or source_table_rec.tableId
+
+  # If that name already exists (as a label), add the source column's name as a suffix.
+  avoid_set = set(c.label for c in ref_table_rec.columns)
+  if reverse_label in avoid_set:
+    return reverse_label + "-" + (col_rec.label or col_rec.colId)
+  return reverse_label

sandbox/grist/schema.py

@@ -15,7 +15,7 @@ import six
 
 import actions
 
-SCHEMA_VERSION = 42
+SCHEMA_VERSION = 43
 
 def make_column(col_id, col_type, formula='', isFormula=False):
   return {
@@ -88,6 +88,10 @@ def schema_create_actions():
       # Points to formula columns that hold conditional formatting rules.
       make_column("rules",       "RefList:_grist_Tables_column"),
 
+      # For Ref/RefList columns only, points to the corresponding reverse reference column.
+      # This column will get automatically set to reverse of the references in reverseCol.
+      make_column("reverseCol",       "Ref:_grist_Tables_column"),
+
       # Instructions when to recalculate the formula on a column with isFormula=False (previously
       # known as a "default formula"). Values are RecalcWhen constants defined below.
       make_column("recalcWhen",       "Int"),
@@ -379,17 +383,25 @@ class RecalcWhen(object):
 # These are little structs to represent the document schema that's used in code generation.
 # Schema itself (as stored by Engine) is an OrderedDict(tableId -> SchemaTable), with
 # SchemaTable.columns being an OrderedDict(colId -> SchemaColumn).
+# Note: reverseColId produces types like grist.ReferenceList("Table", reverse_of="ColId")
+# used for two-way references.
 SchemaTable = namedtuple('SchemaTable', ('tableId', 'columns'))
-SchemaColumn = namedtuple('SchemaColumn', ('colId', 'type', 'isFormula', 'formula'))
+SchemaColumn = namedtuple('SchemaColumn', ('colId', 'type', 'isFormula', 'formula', 'reverseColId'))
 
 # Helpers to convert between schema structures and dicts used in schema actions.
 def dict_to_col(col, col_id=None):
   """Convert dict as used in AddColumn/AddTable actions to a SchemaColumn object."""
-  return SchemaColumn(col_id or col["id"], col["type"], bool(col["isFormula"]), col["formula"])
+  return SchemaColumn(col_id or col["id"], col["type"], bool(col["isFormula"]), col["formula"],
+      col.get("reverseColId"))
 
-def col_to_dict(col, include_id=True):
-  """Convert SchemaColumn to dict to use in AddColumn/AddTable actions."""
+def col_to_dict(col, include_id=True, include_default=False):
+  """
+  Convert SchemaColumn to dict to use in AddColumn/AddTable actions.
+  Set include_default=True to include default values explicitly, e.g. override previous values.
+  """
   ret = {"type": col.type, "isFormula": col.isFormula, "formula": col.formula}
+  if col.reverseColId or include_default:
+    ret["reverseColId"] = col.reverseColId
   if include_id:
     ret["id"] = col.colId
   return ret
@@ -406,6 +418,14 @@ def clone_schema(schema):
   return OrderedDict((t, SchemaTable(s.tableId, s.columns.copy()))
                      for (t, s) in six.iteritems(schema))
 
+def get_reverse_col_id_lookup_func(collist):
+  """
+  Given a list of _grist_Tables_column records, return a function that takes a record and
+  returns its reverseColId.
+  """
+  col_ref_to_col_id = {c.id: c.colId for c in collist}
+  return lambda c: col_ref_to_col_id.get(getattr(c, 'reverseCol', 0))
+
 def build_schema(meta_tables, meta_columns, include_builtin=True):
   """
   Arguments are TableData objects for the _grist_Tables and _grist_Tables_column tables.
@@ -425,8 +445,12 @@ def build_schema(meta_tables, meta_columns, include_builtin=True):
                    key=lambda c: (c.parentId, c.parentPos))
   coldict = {t: list(cols) for t, cols in itertools.groupby(collist, lambda r: r.parentId)}
 
+  # Translate reverseCol in metadata to reverseColId in schema structure.
+  reverse_col_id = get_reverse_col_id_lookup_func(collist)
+
   for t in actions.transpose_bulk_action(meta_tables):
-    columns = OrderedDict((c.colId, SchemaColumn(c.colId, c.type, bool(c.isFormula), c.formula))
-                          for c in coldict[t.id])
+    columns = OrderedDict(
+        (c.colId, SchemaColumn(c.colId, c.type, bool(c.isFormula), c.formula, reverse_col_id(c)))
+        for c in coldict[t.id])
     schema[t.tableId] = SchemaTable(t.tableId, columns)
   return schema

sandbox/grist/table.py

@@ -229,6 +229,10 @@ class Table(object):
     # Set of ReferenceColumn objects that refer to this table
     self._back_references = set()
 
+    # Maps the depend.Node of the source column (possibly in a different table) to column(s) in
+    # this table that have that source column as reverseColRef: {Node: [Column, ...]}.
+    self._reverse_cols_by_source_node = {}
+
     # Store the constant Node for "new columns". Accessing invalid columns creates a dependency
     # on this node, and triggers recomputation when columns are added or renamed.
     self._new_columns_node = depend.Node(self.table_id, None)

sandbox/grist/test_gencode.py

@@ -90,6 +90,7 @@ class TestGenCode(unittest.TestCase):
     gcode = gencode.GenCode()
     gcode.make_module(self.schema)
     module = gcode.usercode
+    # pylint: disable=E1101
     self.assertTrue(isinstance(module.Students, table.UserTable))
 
   def test_ident_combining_chars(self):
@@ -242,7 +243,7 @@ class TestGenCode(unittest.TestCase):
     # Test the case of a bare-word function with a keyword argument appearing in a formula. This
     # case had a bug with code parsing.
     self.schema['Address'].columns['testcol'] = schema.SchemaColumn(
-      'testcol', 'Any', True, 'foo(bar=$region) or max(Students.all, key=lambda n: -n)')
+      'testcol', 'Any', True, 'foo(bar=$region) or max(Students.all, key=lambda n: -n)', None)
     gcode.make_module(self.schema)
     self.assertEqual(gcode.grist_names(), expected_names + [
       (('Address', 'testcol'), 9, 'Address', 'region'),

sandbox/grist/test_reflist_rel.py

@@ -1,8 +1,6 @@
-"""
-This test replicates a bug involving a column conversion after a table rename in the presence of
-a RefList. A RefList column type today only appears as a result of detaching a summary table.
-"""
+import json
 import logging
+import unittest
 import test_engine
 from test_engine import Table, Column
 
@@ -10,6 +8,10 @@ log = logging.getLogger(__name__)
 
 class TestRefListRelation(test_engine.EngineTestCase):
   def test_ref_list_relation(self):
+    """
+    This test replicates a bug involving a column conversion after a table rename in the presence of
+    a RefList. A RefList column type today only appears as a result of detaching a summary table.
+    """
     # Create two tables, the second referring to the first using a RefList and a Ref column.
     self.apply_user_action(["AddTable", "TableA", [
       {"id": "ColA", "type": "Text"}
@@ -100,3 +102,69 @@ class TestRefListRelation(test_engine.EngineTestCase):
       [ 2,      2,    "b",                    [],     0 ],
       [ 3,      1,    "a",                    [],     0 ],
     ])
+
+
+  def test_ref_list_conversion_from_string(self):
+    """
+    RefLists can accept JSON arrays as strings, but only if they look valid.
+    This feature is used by 2 way references, and column renames where type of the column
+    is changed briefly to Int (or other) and the value is converted to string (to represent
+    an error), then when column recovers its type, it should be able to read this string
+    and restore its value
+    """
+    self.apply_user_action(["AddTable", "Tree", [
+      {"id": "Name", "type": "Text"},
+      {"id": "Children", "type": "RefList:Tree"},
+    ]])
+
+    # Add two records.
+    self.apply_user_action(["BulkAddRecord", "Tree", [None]*2,
+      {"Name": ["John", "Bobby"]}])
+
+
+    test_literal = lambda x: self.assertTableData('Tree', cols="subset", data=[
+      [ "id", "Name", "Children" ],
+      [ 1,    "John", x],
+      [ 2,    "Bobby", None ],
+    ])
+
+    invalid_json_arrays = (
+      '["Bobby"]',
+      '["2"]',
+      '["2", "3"]',
+      '[-1]',
+      '["1", "-1"]',
+      '[0]',
+    )
+
+    for value in invalid_json_arrays:
+      self.apply_user_action(
+        ['UpdateRecord', 'Tree', 1, {'Children': value}]
+      )
+      test_literal(value)
+
+    valid_json_arrays = (
+      '[2]',
+      '[1, 2]',
+      '[100]',
+    )
+
+    for value in valid_json_arrays:
+      # Clear value
+      self.apply_user_action(
+        ['UpdateRecord', 'Tree', 1, {'Children': None}]
+      )
+      self.apply_user_action(
+        ['UpdateRecord', 'Tree', 1, {'Children': value}]
+      )
+      self.assertTableData('Tree', cols="subset", data=[
+        [ "id", "Name", "Children" ],
+        [ 1,    "John", json.loads(value) ],
+        [ 2,    "Bobby", None ],
+      ])
+
+
+
+
+if __name__ == "__main__":
+  unittest.main()

sandbox/grist/useractions.py

@@ -10,9 +10,9 @@ import six
 from six.moves import xrange
 
 import acl
+from acl import parse_acl_formulas
 import depend
 import gencode
-from acl import parse_acl_formulas
 from dropdown_condition import parse_dropdown_conditions
 import dropdown_condition
 import actions
@@ -20,12 +20,14 @@ import column
 import sort_specs
 import identifiers
 from objtypes import strict_equal, encode_object
+from reverse_references import check_desired_reverse_col, pick_reverse_col_label
 import schema
 from schema import RecalcWhen
 import summary
 import import_actions
 import textbuilder
 import usertypes
+from usertypes import get_referenced_table_id, get_pure_type, is_compatible_ref_type
 import treeview
 
 from table import get_validation_func_name
@@ -51,7 +53,7 @@ _inherited_groupby_col_fields = {'colId', 'widgetOptions', 'label', 'untieColIdF
 _inherited_summary_col_fields = {'colId', 'label'}
 
 # Schema properties that can be modified using ModifyColumn docaction.
-_modify_col_schema_props = {'type', 'formula', 'isFormula'}
+_modify_col_schema_props = {'type', 'formula', 'isFormula', 'reverseColId'}
 
 
 # A few generic helpers.
@@ -548,8 +550,9 @@ class UserActions(object):
 
   @useraction
   def BulkUpdateRecord(self, table_id, row_ids, columns):
-    columns = actions.decode_bulk_values(columns)
+    return self._BulkUpdateRecord_decoded(table_id, row_ids, actions.decode_bulk_values(columns))
 
+  def _BulkUpdateRecord_decoded(self, table_id, row_ids, columns):
     # Handle special tables, updates to which imply metadata actions.
 
     # Check that the update is valid.
@@ -664,6 +667,8 @@ class UserActions(object):
 
   @override_action('BulkUpdateRecord', '_grist_Tables_column')
   def _updateColumnRecords(self, table_id, row_ids, col_values):
+    # pylint: disable=too-many-statements
+
     # Does various automatic adjustments required for column updates.
     # col_values is a dict of arrays, each array containing values for all col_recs. We process
     # each column individually (to keep code simpler), in _adjust_one_column_update.
@@ -708,6 +713,14 @@ class UserActions(object):
       for col_rec, new_formula in sorted(six.iteritems(formula_updates)):
         col_updates.setdefault(col_rec, {}).setdefault('formula', new_formula)
 
+      # For any renames of columns that have a reverse, tag their reverse column as
+      # changing, so that we can update its schema.
+      for c, values in list(col_updates.items()):
+        reverse_col_ref = values.get('reverseCol', c.reverseCol.id)
+        if has_diff_value(values, 'colId', c.colId) and reverse_col_ref:
+          rcol_rec = self._docmodel.columns.table.get_record(reverse_col_ref)
+          col_updates.setdefault(rcol_rec, {}).setdefault('reverseCol', c.id)
+
     update_pairs = col_updates.items()
 
     # Disallow most changes to summary group-by columns, except to match the underlying column.
@@ -717,6 +730,10 @@ class UserActions(object):
       if col.summarySourceCol:
         underlying_updates = col_updates.get(col.summarySourceCol, {})
         for key, value in six.iteritems(values):
+          if key == 'summarySourceCol' and not value and not col.summarySourceCol._exists():
+            # We are unsetting summarySourceCol because it no longer exists. That's fine; the
+            # record we are updating is actually also about to be deleted.
+            continue
           if key in ('displayCol', 'visibleCol'):
             # These can't always match the underlying column, and can now be changed in the
             # group-by column. (Perhaps the same should be permitted for all widget options.)
@@ -735,6 +752,18 @@ class UserActions(object):
     for c, values in update_pairs:
       # Trigger ModifyColumn and RenameColumn as necessary
       schema_colinfo = select_keys(values, _modify_col_schema_props)
+
+      # If we set reverseCol in metadata, that turns into reverseColId in schema (which is used to
+      # generate Python code). Note that _adjust_one_column_update has already done sanity checks.
+      if 'reverseCol' in values:
+        reverse_col_ref = values['reverseCol']
+        if reverse_col_ref:
+          reverse_col = self._docmodel.columns.table.get_record(reverse_col_ref)
+          reverse_updates = col_updates.get(reverse_col, {})
+          schema_colinfo['reverseColId'] = reverse_updates.get('colId', reverse_col.colId)
+        else:
+          schema_colinfo['reverseColId'] = None
+
       if schema_colinfo:
         self.doModifyColumn(c.parentId.tableId, c.colId, schema_colinfo)
       if has_diff_value(values, 'colId', c.colId):
@@ -743,9 +772,10 @@ class UserActions(object):
           rename_summary_tables.add(c.parentId)
 
     # If we change a column's type, we should ALSO unset each affected field's displayCol.
-    type_changed = [c for c, values in update_pairs if has_diff_value(values, 'type', c.type)]
+    type_changed = [c for c, values in update_pairs if has_diff_value(values, 'type', c.type)
+                      and not is_compatible_ref_type(values.get('type', c.type), c.type)]
     self._docmodel.update([f for c in type_changed for f in c.viewFields],
-                          displayCol=0)
+                          displayCol=0, visibleCol=0)
 
     self.doBulkUpdateFromPairs(table_id, update_pairs)
 
@@ -868,14 +898,38 @@ class UserActions(object):
       col_values['type'] = guess_type(self._get_column_values(col), convert=False)
 
     # If changing the type of a column, unset its displayCol by default.
-    if 'type' in col_values:
+    new_type = col_values.get('type', col.type)
+    if 'type' in col_values and not is_compatible_ref_type(new_type, col.type):
       col_values.setdefault('displayCol', 0)
+      col_values.setdefault('visibleCol', 0)
 
     # Collect all updates for dependent summary columns.
     results = []
     def add(cols, value_dict):
       results.extend((c, summary.skip_rules_update(c, value_dict)) for c in cols)
 
+    # If changing reverseCol, do some sanity checks and update its counterpart.
+    if has_diff_value(col_values, 'reverseCol', col.reverseCol):
+      reverse_col_ref = col_values['reverseCol']
+
+      if col.reverseCol and (col.reverseCol.id in self._docmodel.columns.table.row_ids):
+        # If unsetting (or changing) reverseCol, unset the counterpart that was pointing back.
+        # The existence check above is to handle case when reverseCol is what just got deleted.
+        results.append((col.reverseCol, {'reverseCol': 0}))
+
+      if reverse_col_ref:
+        # If setting new reverseCol, set its counterpart pointing back to us.
+        rcol_rec = self._docmodel.columns.table.get_record(reverse_col_ref)
+        check_desired_reverse_col(new_type, rcol_rec)
+        results.append((rcol_rec, {'reverseCol': col.id}))
+
+    # If a column has a reverseCol, we restrict some changes to it while reverseCol is set.
+    if col_values.get('reverseCol', col.reverseCol):
+      if not is_compatible_ref_type(new_type, col.type):
+        raise ValueError("invalid change to type of a two-way reference column")
+      if col_values.get('formula'):
+        raise ValueError("cannot set formula on a two-way reference column")
+
     source_table = col.parentId.summarySourceTable
     if source_table:  # This is a summary-table column.
       # Disallow isFormula changes.
@@ -1096,10 +1150,13 @@ class UserActions(object):
         continue
       updates = ref_col.get_updates_for_removed_target_rows(row_id_set)
       if updates:
-        self._do_doc_action(actions.BulkUpdateRecord(ref_col.table_id,
+        # Previously we sent this as a docaction. Now we do a proper useraction with all the
+        # processing that involves, e.g. triggering two-way-reference updates, and also all the
+        # metadata checks and updates.
+        self._BulkUpdateRecord_decoded(ref_col.table_id,
           [row_id for (row_id, value) in updates],
           { ref_col.col_id: [value for (row_id, value) in updates] }
-        ))
+        )
 
   @useraction
   def RemoveRecord(self, table_id, row_id):
@@ -1134,6 +1191,11 @@ class UserActions(object):
   def _removeTableRecords(self, table_id, row_ids):
     remove_table_recs = [rec for i, rec in self._bulk_action_iter(table_id, row_ids)]
 
+    # If there are any two-way reference columns in this table, break the connection.
+    cols = (c for t in remove_table_recs for c in t.columns if c.reverseCol)
+    if cols:
+      self._docmodel.update(cols, reverseCol=0)
+
     # If there are summary tables based on this table, remove those too.
     remove_table_recs.extend(st for t in remove_table_recs for st in t.summaryTables)
 
@@ -1190,7 +1252,7 @@ class UserActions(object):
     if not (visible_col and display_col):
       # If there's no visible/display column, we just keep row IDs.
       if col.type.startswith("Ref:"):
-        self.ModifyColumn(table_id, col_id, dict(type="Int"))
+        self.ModifyColumn(table_id, col_id, {"type": "Int", "reverseCol": 0})
       else:
         # Data columns can't be of type Any, and there's no type that can
         # hold a list of numbers. So we convert the lists of row IDs
@@ -1198,7 +1260,7 @@ class UserActions(object):
         # We need to get the values before changing the column type.
         table = self._engine.tables[table_id]
         new_values = [",".join(map(str, row or [])) for row in self._get_column_values(col)]
-        self.ModifyColumn(table_id, col_id, dict(type="Text"))
+        self.ModifyColumn(table_id, col_id, {"type": "Text", "reverseCol": 0})
         self.BulkUpdateRecord(table_id, list(table.row_ids), {col_id: new_values})
       return
 
@@ -1600,7 +1662,7 @@ class UserActions(object):
     to_formula = bool(col_info.get('isFormula', from_formula))
 
     old_col_info = schema.col_to_dict(self._engine.schema[table_id].columns[col_id],
-                                      include_id=False)
+                                      include_id=False, include_default=True)
 
     col_info = {k: v for k, v in six.iteritems(col_info) if old_col_info.get(k, v) != v}
     if not col_info:
@@ -1656,6 +1718,11 @@ class UserActions(object):
       finally:
         self._engine.out_actions.undo.append(mod_action)
 
+    # Give two-way reference columns a chance to get rebuilt after a Ref<>RefList switch.
+    if 'type' in col_info:
+      update_action = new_column.recalc_from_reverse_values()
+      self._do_doc_action(update_action)
+
   @useraction
   def ConvertFromColumn(self, table_id, src_col_id, dst_col_id, typ, widgetOptions, visibleColRef):
     from sandbox import call_external
@@ -1749,8 +1816,7 @@ class UserActions(object):
         changed_values.append(src_value)
 
     # Produce the BulkUpdateRecord update.
-    self._do_doc_action(actions.BulkUpdateRecord(table_id, changed_rows,
-                                                 {dst_col_id: changed_values}))
+    self._BulkUpdateRecord_decoded(table_id, changed_rows, {dst_col_id: changed_values})
 
   @useraction
   def MaybeCopyDisplayFormula(self, src_col_ref, dst_col_ref):
@@ -1843,6 +1909,66 @@ class UserActions(object):
     updated_rules = existing_rules + [new_rule]
     self._docmodel.update([rule_owner], rules=[encode_object(updated_rules)])
 
+  @useraction
+  def AddReverseColumn(self, table_id, col_id):
+    """
+    Adds a reverse reference column corresponding to `col_id`. This creates a two-way binding
+    between two Ref/RefList columns. Updating one of them will result in updating the other. To
+    break the binding, one of the columns should be removed (using a regular DocAction).
+
+    If a Foo column (Ref:Table1) has a reverse Bar column (RefList:Table2), then updating the Foo
+    column with a doc action like:
+      ['UpdateRecord', 'Table2', 1, {'Foo': 2}]
+    will result in updating the Bar column with a "back reference" like:
+      ['UpdateRecord', 'Table1', 2, {'Bar': ['L', 1]}]
+
+    By default, the type of the reverse column added is RefList, as the column `col_id` might have
+    multiple references (duplicated data). To properly represent it, the reverse column must be of
+    RefList type. The user can change the type of both columns (or either one) to Ref type, but the
+    engine will prevent it if one of the columns has duplicated values (more than one row in Table1
+    points to the same row in Table2).
+
+    The binding is symmetric. There is no "primary" or "secondary" column. Both columns are equal,
+    and the user can remove either of them and recreate it later from the other one.
+    """
+    col_rec = self._docmodel.get_column_rec(table_id, col_id)
+    if col_rec.reverseCol:
+      raise ValueError('reverse reference column already exists')
+    target_table_id = get_referenced_table_id(col_rec.type)
+    if not target_table_id:
+      raise ValueError('reverse column can only be added to a reference column')
+
+    reverse_label = pick_reverse_col_label(self._docmodel, col_rec)
+    ret = self.AddVisibleColumn(target_table_id, reverse_label, {
+      "isFormula": False,
+      "type": "RefList:" + table_id,
+    })
+    added_col = self._docmodel.columns.table.get_record(ret['colRef'])
+    self._docmodel.update([col_rec], reverseCol=added_col.id)
+    self._pick_and_set_display_col(added_col)
+
+    # Fill in the new column.
+    col_obj = self._docmodel.get_table(table_id).table.get_column(col_id)
+    update_action = col_obj.recalc_from_reverse_values()
+    self._do_doc_action(update_action)
+
+    return ret
+
+  def _pick_and_set_display_col(self, col_rec):
+    target_table_id = get_referenced_table_id(col_rec.type)
+    target_table_rec = self._docmodel.get_table_rec(target_table_id)
+
+    # Types that could conceivably be identifiers for a record (this is very loose, but at
+    # least excludes types like references and attachments).
+    maybe_ident_types = ['Text', 'Any', 'Numeric', 'Int', 'Date', 'DateTime', 'Choice']
+
+    # Use the first column from target table, if it's a reasonable type.
+    for vcol in target_table_rec.columns:
+      if column.is_visible_column(vcol.colId) and get_pure_type(vcol.type) in maybe_ident_types:
+        self._docmodel.update([col_rec], visibleCol=vcol.id)
+        self.SetDisplayFormula(col_rec.tableId, None, col_rec.id,
+            '$%s.%s' % (col_rec.colId, vcol.colId))
+        break
 
   #----------------------------------------
   # User actions on tables.

sandbox/grist/usertypes.py

@@ -20,6 +20,7 @@ import math
 
 import six
 from six import integer_types
+import depend
 import objtypes
 from objtypes import AltText, is_int_short
 import moment
@@ -49,9 +50,14 @@ _type_defaults = {
   'Text':         u'',
 }
 
+def get_pure_type(col_type):
+  """
+  Returns type to the first colon, i.e. strips suffix for Ref:, DateTime:, etc.
+  """
+  return col_type.split(':', 1)[0]
+
 def get_type_default(col_type):
-  col_type = col_type.split(':', 1)[0]      # Strip suffix for Ref:, DateTime:, etc.
-  return _type_defaults.get(col_type, None)
+  return _type_defaults.get(get_pure_type(col_type), None)
 
 def formulaType(grist_type):
   """
@@ -70,6 +76,13 @@ def get_referenced_table_id(col_type):
     return col_type[8:]
   return None
 
+def is_compatible_ref_type(type1, type2):
+  """
+  Returns whether type1 and type2 are Ref or RefList types with the same target table.
+  """
+  ref_table1 = get_referenced_table_id(type1)
+  ref_table2 = get_referenced_table_id(type2)
+  return bool(ref_table1 and ref_table1 == ref_table2)
 
 def ifError(value, value_if_error):
   """
@@ -415,27 +428,36 @@ class Reference(Id):
   record is available as `rec.foo._row_id`. It is equivalent to `rec.foo.id`, except that
   accessing `id`, as other public properties, involves a lookup in `Foo` table.
   """
-  def __init__(self, table_id):
+  def __init__(self, table_id, reverse_of=None):
     super(Reference, self).__init__()
     self.table_id = table_id
+    self._reverse_col_id = reverse_of
 
   @classmethod
   def typename(cls):
     return "Ref"
 
+  def reverse_source_node(self):
+    """ Returns the reverse column as depend.Node, if it exists. """
+    return depend.Node(self.table_id, self._reverse_col_id) if self._reverse_col_id else None
 
 class ReferenceList(BaseColumnType):
   """
   ReferenceList stores a list of references into another table.
   """
-  def __init__(self, table_id):
+  def __init__(self, table_id, reverse_of=None):
     super(ReferenceList, self).__init__()
     self.table_id = table_id
+    self._reverse_col_id = reverse_of
 
   @classmethod
   def typename(cls):
     return "RefList"
 
+  def reverse_source_node(self):
+    """ Returns the reverse column as depend.Node, if it exists. """
+    return depend.Node(self.table_id, self._reverse_col_id) if self._reverse_col_id else None
+
   def do_convert(self, value):
     if isinstance(value, six.string_types):
       # This is second part of a "hack" we have to do when we rename tables. During
@@ -448,7 +470,11 @@ class ReferenceList(BaseColumnType):
       try:
         # If it's a string that looks like JSON, try to parse it as such.
         if value.startswith('['):
-          value = json.loads(value)
+          parsed = json.loads(value)
+          # It must be list of integers, and all of them must be positive integers.
+          if (isinstance(parsed, list) and
+              all(isinstance(v, int) and v > 0 for v in parsed)):
+            value = parsed
         else:
         # Else try to parse it as a RecordList
           value = objtypes.RecordList.from_repr(value)