(core) Fix recursion error in invalidate_deps

Summary: Convert recursion to while loop with stack of things to invalidate.

Test Plan: Loan tracker example works now

Reviewers: dsagal

Reviewed By: dsagal

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

sandbox/grist/depend.py

@@ -131,25 +131,35 @@ class Graph(object):
     If dirty_rows is ALL_ROWS, the whole column is affected, and dependencies get recomputed from
     scratch. ALL_ROWS propagates to all dependent columns, so those also get recomputed in full.
     """
-    if include_self:
+    to_invalidate = [(dirty_node, dirty_rows)]
-      if recompute_map.get(dirty_node) == ALL_ROWS:
-        return
-      if dirty_rows == ALL_ROWS:
-        recompute_map[dirty_node] = ALL_ROWS
-        # If all rows are being recomputed, clear the dependencies of the affected column. (We add
-        # dependencies in the course of recomputing, but we can only start from an empty set of
-        # dependencies if we are about to recompute all rows.)
-        self.clear_dependencies(dirty_node)
-      else:
-        out_rows = recompute_map.setdefault(dirty_node, SortedSet())
-        prev_count = len(out_rows)
-        out_rows.update(dirty_rows)
-        # Don't bother recursing into dependencies if we didn't actually update anything.
-        if len(out_rows) <= prev_count:
-          return
 
-    # Iterate through a copy of _in_node_map, because recursive clear_dependencies may modify it.
+    while to_invalidate:
-    for edge in list(self._in_node_map.get(dirty_node, ())):
+      dirty_node, dirty_rows = to_invalidate.pop()
-      affected_rows = (ALL_ROWS if dirty_rows == ALL_ROWS else
+      if include_self:
-                       edge.relation.get_affected_rows(dirty_rows))
+        if recompute_map.get(dirty_node) == ALL_ROWS:
-      self.invalidate_deps(edge.out_node, affected_rows, recompute_map, include_self=True)
+          continue
+        if dirty_rows == ALL_ROWS:
+          recompute_map[dirty_node] = ALL_ROWS
+          # If all rows are being recomputed, clear the dependencies of the affected column. (We add
+          # dependencies in the course of recomputing, but we can only start from an empty set of
+          # dependencies if we are about to recompute all rows.)
+          self.clear_dependencies(dirty_node)
+        else:
+          out_rows = recompute_map.setdefault(dirty_node, SortedSet())
+          prev_count = len(out_rows)
+          out_rows.update(dirty_rows)
+          # Don't bother recursing into dependencies if we didn't actually update anything.
+          if len(out_rows) <= prev_count:
+            continue
+
+      include_self = True
+
+      for edge in self._in_node_map.get(dirty_node, ()):
+        affected_rows = (ALL_ROWS if dirty_rows == ALL_ROWS else
+                         edge.relation.get_affected_rows(dirty_rows))
+
+        # Previously this was:
+        #   self.invalidate_deps(edge.out_node, affected_rows, recompute_map, include_self=True)
+        # but that led to a recursion error, so now we do the equivalent
+        # without actual recursion, hence the while loop
+        to_invalidate.append((edge.out_node, affected_rows))

sandbox/grist/test_depend.py

@@ -0,0 +1,44 @@
+import testutil
+import test_engine
+
+class TestDependencies(test_engine.EngineTestCase):
+  sample_desc = {
+    "SCHEMA": [
+      [1, "Table1", [
+        [1, "Prev",       "Ref:Table1",  True, "Table1.lookupOne(id=$id-1)", "", ""],
+        [2, "Value",      "Numeric",     False, "", "", ""],
+        [3, "Sum",        "Numeric",     True, "($Prev.Sum or 0) + $Value", "", ""],
+      ]]
+    ],
+    "DATA": {
+      "Table1": [
+        ["id","Value"],
+      ] + [[n + 1, n + 1] for n in range(3200)]
+    }
+  }
+
+  def test_recursive_column_dependencies(self):
+    sample = testutil.parse_test_sample(self.sample_desc)
+    self.load_sample(sample)
+    self.apply_user_action(['Calculate'])
+
+    # The Sum column contains a cumulative total of the Value column
+    self.assertTableData("Table1", cols="subset", rows="subset", data=[
+      ["id", "Value", "Sum"],
+      [1,    1,       1],
+      [2,    2,       3],
+      [3,    3,       6],
+      [3200, 3200,    5121600],
+    ])
+
+    # Updating the first Value causes a cascade of changes to Sum,
+    # invalidating dependencies one cell at a time.
+    # Previously this cause a recursion error.
+    self.update_record("Table1", 1, Value=11)
+    self.assertTableData("Table1", cols="subset", rows="subset", data=[
+      ["id", "Value", "Sum"],
+      [1,    11,      11],
+      [2,    2,       13],
+      [3,    3,       16],
+      [3200, 3200,    5121610],
+    ])