From 392fe5eab4d7e43ced5b38b5b9b250e7a849220c Mon Sep 17 00:00:00 2001
From: "Michael J. Sullivan" <sully@msully.net>
Date: Tue, 23 Jan 2024 21:32:59 -0800
Subject: [PATCH] Fix issue with schema changes when trigger depends on a
 rewrite (#6706)

Dependencies on rewrites, policies, and triggers don't get explicitly
tracked as references, but the things they depend on transitively
through those do.

The dummy expression for rewrites is not the correct type, so if a
trigger and a rewrite are being reprocessed in _propagate_if_expr refs
at the same time, and the trigger uses the rewrite, and is processed
first, then it will error.
Fix this by having the rewrite dummy expression have the right type.

I've experimented with having rewrites and triggers explicitly tracked
as dependencies, but it complicates some code and I don't think it
actually fixes any real problems.
---
 edb/schema/rewrites.py   | 21 ++++++++++++++-------
 tests/test_edgeql_ddl.py | 27 +++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/edb/schema/rewrites.py b/edb/schema/rewrites.py
index 681b5fd1995..5ddb290f10d 100644
--- a/edb/schema/rewrites.py
+++ b/edb/schema/rewrites.py
@@ -36,6 +36,9 @@
 from . import schema as s_schema
 from . import types as s_types
 
+if TYPE_CHECKING:
+    from . import pointers as s_pointers
+
 
 class Rewrite(
     referencing.NamedReferencedInheritingObject,
@@ -70,6 +73,13 @@ def should_propagate(self, schema: s_schema.Schema) -> bool:
         # when retrieving them.
         return False
 
+    def get_ptr_target(self, schema: s_schema.Schema) -> s_types.Type:
+        pointer: s_pointers.Pointer = cast(
+            's_pointers.Pointer', self.get_subject(schema))
+        ptr_target = pointer.get_target(schema)
+        assert ptr_target
+        return ptr_target
+
 
 class RewriteCommandContext(
     sd.ObjectCommandContext[Rewrite],
@@ -223,7 +233,9 @@ def get_dummy_expr_field_value(
         value: Any,
     ) -> Optional[s_expr.Expression]:
         if field.name == 'expr':
-            return s_expr.Expression(text='false')
+            pt = self.scls.get_ptr_target(schema)
+            text = f'<{pt.get_displayname(schema)}>{{}}'
+            return s_expr.Expression(text=text)
         else:
             raise NotImplementedError(f'unhandled field {field.name!r}')
 
@@ -232,8 +244,6 @@ def validate_object(
         schema: s_schema.Schema,
         context: sd.CommandContext,
     ) -> None:
-        from . import pointers as s_pointers
-
         expr: s_expr.Expression = self.scls.get_expr(schema)
 
         if not expr.irast:
@@ -262,10 +272,7 @@ def validate_object(
                 context=source_context,
             )
 
-        pointer = self.scls.get_subject(compiled_schema)
-        assert isinstance(pointer, s_pointers.Pointer)
-        ptr_target = pointer.get_target(compiled_schema)
-        assert ptr_target
+        ptr_target = self.scls.get_ptr_target(compiled_schema)
         if not typ.assignment_castable_to(ptr_target, compiled_schema):
             source_context = self.get_attribute_source_context('expr')
             raise errors.SchemaDefinitionError(
diff --git a/tests/test_edgeql_ddl.py b/tests/test_edgeql_ddl.py
index 7871993b36c..cb19712f06c 100644
--- a/tests/test_edgeql_ddl.py
+++ b/tests/test_edgeql_ddl.py
@@ -13242,6 +13242,33 @@ async def test_edgeql_ddl_change_module_04(self):
             DROP TYPE foo::Tag;
         """)
 
+    async def test_edgeql_ddl_rewrite_and_trigger_01(self):
+        await self.con.execute("""
+            create type Entry {
+                create property x := 0;
+                create property y -> int64 {
+                    create rewrite insert, update using (.x);
+                };
+            };
+            create type Foo {
+                create trigger log0 after insert for each do (insert Entry);
+            };
+            create type Bar {
+                create trigger log1 after insert for each do (insert Foo);
+            };
+        """)
+
+        await self.con.execute(f"""
+            alter type Entry alter property x using (1)
+        """)
+
+        await self.con.execute(f"""
+            alter type Entry alter property y drop rewrite insert, update;
+        """)
+        await self.con.execute(f"""
+            alter type Foo drop trigger log0;
+        """)
+
     async def _simple_rename_ref_test(
         self,
         ddl,