diff --git a/sandbox/grist/depend.py b/sandbox/grist/depend.py index 41be2e1e..2408a4b0 100644 --- a/sandbox/grist/depend.py +++ b/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: - 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 + to_invalidate = [(dirty_node, dirty_rows)] - # Iterate through a copy of _in_node_map, because recursive clear_dependencies may modify it. - for edge in list(self._in_node_map.get(dirty_node, ())): - affected_rows = (ALL_ROWS if dirty_rows == ALL_ROWS else - edge.relation.get_affected_rows(dirty_rows)) - self.invalidate_deps(edge.out_node, affected_rows, recompute_map, include_self=True) + while to_invalidate: + dirty_node, dirty_rows = to_invalidate.pop() + if include_self: + if recompute_map.get(dirty_node) == ALL_ROWS: + 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)) diff --git a/sandbox/grist/test_depend.py b/sandbox/grist/test_depend.py new file mode 100644 index 00000000..2823d6de --- /dev/null +++ b/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], + ])