Fix exponential time (in inheritance depth) migrations when altering things (#8671)

The issue is that we are propagating changes to *all descendants* (not
all children), but each descendant does the same.

There is a mechanism involving setting 'implicit_propagation' as an
annotation that solves this in other cases; use it in some more cases.

Fixes #7195.

Author: msullivan

edb/edgeql/declarative.py

@@ -596,6 +596,15 @@ def trace_layout_CreateProperty(
     _trace_item_layout(node, ctx=ctx)
 
 
+@trace_layout.register
+def trace_layout_CreateConstraint(
+    node: qlast.CreateConstraint,
+    *,
+    ctx: LayoutTraceContext,
+) -> None:
+    _trace_item_layout(node, ctx=ctx)
+
+
 def _trace_item_layout(
     node: qlast.CreateObject,
     *,

edb/schema/constraints.py

@@ -730,6 +730,39 @@ def localnames_from_ast(
 
         return localnames
 
+    def inherit_fields(
+        self,
+        schema: s_schema.Schema,
+        context: sd.CommandContext,
+        bases: tuple[so.Object, ...],
+        *,
+        fields: Optional[Iterable[str]] = None,
+        ignore_local: bool = False,
+        apply: bool = True,
+    ) -> s_schema.Schema:
+        # Concrete constraints populate a bunch of other fields that
+        # can be based on their abstract parents but don't come from
+        # them. So if we are inheriting a new expr from a potentially
+        # abstract parent, we need to actually inherit all of these
+        # other properties that can be populated by
+        # _populate_concrete_constraint_attrs.
+        #
+        # This is pretty fragile though, and I don't love it.
+
+        if fields is not None:
+            fields = set(fields) | {
+                'subjectexpr', 'finalexpr', 'abstract', 'args'
+            }
+
+        return super().inherit_fields(
+            schema,
+            context,
+            bases,
+            fields=fields,
+            ignore_local=ignore_local,
+            apply=apply,
+        )
+
     def _populate_concrete_constraint_attrs(
         self,
         schema: s_schema.Schema,

edb/schema/inheriting.py

@@ -959,6 +959,9 @@ def _propagate_field_alter(
         scls: so.InheritingObject,
         props: tuple[str, ...],
     ) -> None:
+        if _has_implicit_propagation(context):
+            return
+
         descendant_names = [
             d.get_name(schema) for d in scls.ordered_descendants(schema)
         ]
@@ -979,6 +982,7 @@ def _propagate_field_alter(
                 d_alter_cmd.add(self.clone(d_alter_cmd.classname))
 
             with ctx_stack():
+                d_alter_cmd.set_annotation('implicit_propagation', True)
                 assert isinstance(d_alter_cmd, InheritingObjectCommand)
                 schema = d_alter_cmd.inherit_fields(
                     schema, context, d_bases, fields=props, apply=False
@@ -1145,12 +1149,16 @@ def _alter_finalize(
 
         if not context.canonical:
             schema = self._recompute_inheritance(schema, context)
-            if context.enable_recursion:
+            if (
+                context.enable_recursion
+                and not _has_implicit_propagation(context)
+            ):
                 for descendant in self.scls.ordered_descendants(schema):
                     d_root_cmd, d_alter_cmd, ctx_stack = (
                         descendant.init_delta_branch(
                             schema, context, sd.AlterObject))
                     assert isinstance(d_alter_cmd, InheritingObjectCommand)
+                    d_alter_cmd.set_annotation('implicit_propagation', True)
                     with ctx_stack():
                         d_alter_cmd._fixup_inheritance_refdicts(
                             schema, context)
@@ -1331,3 +1339,13 @@ def _needs_refdict(refdict: so.RefDict, context: sd.CommandContext) -> bool:
         inheritance_refdicts is None
         or refdict.attr in inheritance_refdicts
     ) and (context.inheritance_merge is None or context.inheritance_merge)
+
+
+def _has_implicit_propagation(context: sd.CommandContext) -> bool:
+    for ctx in reversed(context.stack):
+        if (
+            isinstance(ctx.op, sd.ObjectCommand)
+            and ctx.op.get_annotation('implicit_propagation')
+        ):
+            return True
+    return False

edb/schema/referencing.py

@@ -853,12 +853,8 @@ def _propagate_ref_op(
         scls: ReferencedInheritingObject,
         cb: Callable[[sd.ObjectCommand[so.Object], sn.Name], None]
     ) -> None:
-        for ctx in reversed(context.stack):
-            if (
-                isinstance(ctx.op, sd.ObjectCommand)
-                and ctx.op.get_annotation('implicit_propagation')
-            ):
-                return
+        if inheriting._has_implicit_propagation(context):
+            return
 
         referrer_ctx = self.get_referrer_context(context)
         if referrer_ctx:

tests/test_edgeql_functions.py

@@ -31,7 +31,7 @@
 from edb.tools import test
 
 
-class TestEdgeQLFunctions(tb.QueryTestCase):
+class TestEdgeQLFunctions(tb.DDLTestCase):
 
     SCHEMA = os.path.join(os.path.dirname(__file__), 'schemas',
                           'issues.esdl')

tests/test_schema.py

@@ -7654,6 +7654,31 @@ def test_schema_migrations_equivalence_linkprops_14(self):
             };
         """])
 
+    def test_schema_migrations_equivalence_linkprops_15(self):
+        self._assert_migration_equivalence([r"""
+            abstract link link_with_value {
+                single property value := 'lol';
+                index on (__subject__@value);
+            }
+            type Tgt;
+            type Foo {
+                link l1 extending link_with_value -> Tgt;
+            };
+            type Bar extending Foo;
+            type Baz extending Bar;
+        """, r"""
+            abstract link link_with_value {
+                single property value := 12;
+                index on (__subject__@value);
+            }
+            type Tgt;
+            type Foo {
+                link l1 extending link_with_value -> Tgt;
+            };
+            type Bar extending Foo;
+            type Baz extending Bar;
+        """])
+
     def test_schema_migrations_equivalence_annotation_01(self):
         self._assert_migration_equivalence([r"""
             type Base;
@@ -8061,7 +8086,7 @@ def test_schema_migrations_equivalence_constraint_04(self):
         """, r"""
         """])
 
-    def test_schema_migrations_equivalence_constraint_05(self):
+    def test_schema_migrations_equivalence_constraint_05a(self):
         self._assert_migration_equivalence([r"""
             abstract constraint not_bad {
                 using (__subject__ != "bad" and __subject__ != "terrible")
@@ -8086,6 +8111,33 @@ def test_schema_migrations_equivalence_constraint_05(self):
             type Bar extending Foo;
         """])
 
+    def test_schema_migrations_equivalence_constraint_05b(self):
+        self._assert_migration_equivalence([r"""
+            abstract constraint not_bad_1 {
+                using (__subject__ != "bad" and __subject__ != "terrible")
+            }
+            abstract constraint not_bad extending not_bad_1;
+
+            type Foo {
+                property x -> str {
+                    constraint not_bad;
+                }
+            }
+            type Bar extending Foo;
+        """, r"""
+            abstract constraint not_bad_1 {
+                using (__subject__ != "bad" and __subject__ != "terrible")
+            }
+            abstract constraint not_bad extending not_bad_1;
+
+            type Foo {
+                property x -> str {
+                    constraint not_bad;
+                }
+            }
+            type Bar extending Foo;
+        """])
+
     def test_schema_migrations_equivalence_constraint_06(self):
         self._assert_migration_equivalence([r"""
             type Cell {