Skip to content

Commit

Permalink
Fix issue with schema changes when trigger depends on a rewrite (#6706)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
msullivan authored Jan 24, 2024
1 parent 20994d9 commit 392fe5e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 7 deletions.
21 changes: 14 additions & 7 deletions edb/schema/rewrites.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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}')

Expand All @@ -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:
Expand Down Expand Up @@ -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(
Expand Down
27 changes: 27 additions & 0 deletions tests/test_edgeql_ddl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 392fe5e

Please sign in to comment.