From 718461504d5f2f23d686fc9739be59f771dc0c6c Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Tue, 30 Apr 2024 23:09:10 -0700 Subject: [PATCH] Make linkprops (including @source/@target) work in conflict selects (#7284) Exclusive constraints on the linkprops themselves still don't work, though it should be fixable. Fixes #7263 --- edb/edgeql/compiler/conflicts.py | 1 + edb/edgeql/compiler/context.py | 5 ++ edb/edgeql/compiler/inference/cardinality.py | 6 +- edb/edgeql/compiler/setgen.py | 7 ++- edb/edgeql/utils.py | 2 + tests/schemas/constraints.esdl | 5 +- tests/test_constraints.py | 59 +++++++++++++++++--- 7 files changed, 73 insertions(+), 12 deletions(-) diff --git a/edb/edgeql/compiler/conflicts.py b/edb/edgeql/compiler/conflicts.py index 53b8ed4f0a63..e96955a69407 100644 --- a/edb/edgeql/compiler/conflicts.py +++ b/edb/edgeql/compiler/conflicts.py @@ -419,6 +419,7 @@ def _compile_conflict_select( select_ast = qlast.Set(elements=frags) with ctx.new() as ectx: ectx.implicit_limit = 0 + ectx.allow_endpoint_linkprops = True select_ir = dispatch.compile(select_ast, ctx=ectx) select_ir = setgen.scoped_set( select_ir, force_reassign=True, ctx=ectx) diff --git a/edb/edgeql/compiler/context.py b/edb/edgeql/compiler/context.py index e7be6d6045b9..5fe66a8f2b98 100644 --- a/edb/edgeql/compiler/context.py +++ b/edb/edgeql/compiler/context.py @@ -556,6 +556,9 @@ class ContextLevel(compiler.ContextLevel): active_computeds: ordered.OrderedSet[s_pointers.Pointer] """A ordered set of currently compiling computeds""" + allow_endpoint_linkprops: bool + """Whether to allow references to endpoint linkpoints (@source, @target).""" + disallow_dml: Optional[str] """Whether we are currently in a place where no dml is allowed, if not None, then it is of the form `in a FILTER clause` """ @@ -623,6 +626,7 @@ def __init__( self.active_rewrites = frozenset() self.active_defaults = frozenset() + self.allow_endpoint_linkprops = False self.disallow_dml = None else: @@ -666,6 +670,7 @@ def __init__( self.active_rewrites = prevlevel.active_rewrites self.active_defaults = prevlevel.active_defaults + self.allow_endpoint_linkprops = prevlevel.allow_endpoint_linkprops self.disallow_dml = prevlevel.disallow_dml if mode == ContextSwitchMode.SUBQUERY: diff --git a/edb/edgeql/compiler/inference/cardinality.py b/edb/edgeql/compiler/inference/cardinality.py index be55263a58b3..b401cbb0b39c 100644 --- a/edb/edgeql/compiler/inference/cardinality.py +++ b/edb/edgeql/compiler/inference/cardinality.py @@ -945,9 +945,11 @@ def extract_filters( ptr = left_stype.getptr(schema, sn.UnqualName('id')) pointers.append(ptr) else: - left = irutils.unwrap_set(left) - while left.path_id != result_set.path_id: + if irutils.is_implicit_wrapper(left.expr): + left = left.expr.result + continue + assert isinstance(left.expr, irast.Pointer) ptr = env.schema.get( left.expr.ptrref.name, diff --git a/edb/edgeql/compiler/setgen.py b/edb/edgeql/compiler/setgen.py index 3925db91d2a6..90aed4837d46 100644 --- a/edb/edgeql/compiler/setgen.py +++ b/edb/edgeql/compiler/setgen.py @@ -428,8 +428,11 @@ def compile_path(expr: qlast.Path, *, ctx: context.ContextLevel) -> irast.Set: # anyway, so disallow them. if ( ptr_expr.name in ('source', 'target') - and ctx.env.options.schema_object_context - not in (s_constr.Constraint, s_indexes.Index) + and not ctx.allow_endpoint_linkprops + and ( + ctx.env.options.schema_object_context + not in (s_constr.Constraint, s_indexes.Index) + ) ): raise errors.QueryError( f'@{ptr_expr.name} may only be used in index and ' diff --git a/edb/edgeql/utils.py b/edb/edgeql/utils.py index dad5a468d82a..3d703af3b02f 100644 --- a/edb/edgeql/utils.py +++ b/edb/edgeql/utils.py @@ -193,6 +193,8 @@ def subject_substitute( for path in find_paths(ast): if isinstance(path.steps[0], qlast.Subject): path.steps[0] = new_subject + elif path.partial: + path.steps[0:0] = [new_subject] return ast diff --git a/tests/schemas/constraints.esdl b/tests/schemas/constraints.esdl index 5ff688621d65..9ae49d8b4ea1 100644 --- a/tests/schemas/constraints.esdl +++ b/tests/schemas/constraints.esdl @@ -76,8 +76,11 @@ abstract link translated_label { abstract link link_with_unique_property { property unique_property -> str { - constraint exclusive; + # TODO: Move the constraint back here once linkprop constraints + # supported in conflict selects. + # constraint exclusive; } + constraint exclusive on (@unique_property); } abstract link link_with_unique_property_inherited diff --git a/tests/test_constraints.py b/tests/test_constraints.py index 9ecaf4406aea..e6733702e64c 100644 --- a/tests/test_constraints.py +++ b/tests/test_constraints.py @@ -500,8 +500,7 @@ async def test_constraints_endpoint_constraint_01(self): """) async def test_constraints_endpoint_constraint_02(self): - # testing (@source, @lang) on a single link - # This constraint is pointless and can never fail + # testing (@source, @lang) on a multi link await self.con.execute(""" insert UniqueName { translated_labels := ((insert Label { text := "x" }) { @@ -509,6 +508,12 @@ async def test_constraints_endpoint_constraint_02(self): }; """) + await self.con.execute(""" + update UniqueName set { + translated_labels := Label {@lang := 'x' } + }; + """) + # Should be fine await self.con.execute(""" insert UniqueName { @@ -562,7 +567,7 @@ async def test_constraints_endpoint_constraint_03(self): # Same @target different @lang await self.con.execute(""" - insert UniqueName { + insert UniqueNameInherited { translated_label_tgt := ( select Label { @lang := 'y' } filter .text = 'x' limit 1) }; @@ -590,6 +595,26 @@ async def test_constraints_endpoint_constraint_03(self): }; """) + async with self.assertRaisesRegexTx( + edgedb.ConstraintViolationError, + 'violates exclusivity constraint' + ): + await self.con.execute(""" + update UniqueName + filter .translated_label_tgt.text = 'x' + set { translated_label_tgt := + .translated_label_tgt { @lang := '!' } + } + """) + + await self.con.execute(""" + update UniqueName + filter .translated_label_tgt.text = 'x' + set { translated_label_tgt := + .translated_label_tgt { @lang := @lang } + } + """) + async def test_constraints_endpoint_constraint_04(self): # testing (@target, @lang) on a multi link await self.con.execute(""" @@ -616,10 +641,10 @@ async def test_constraints_endpoint_constraint_04(self): """) await self.con.execute(""" - insert UniqueNameInherited { - translated_labels_tgt := ( - select Label { @lang := 'x!' }) - }; + insert UniqueNameInherited { + translated_labels_tgt := ( + select Label { @lang := 'x!' }) + }; """) async with self.assertRaisesRegexTx( @@ -656,6 +681,26 @@ async def test_constraints_endpoint_constraint_04(self): }; """) + async with self.assertRaisesRegexTx( + edgedb.ConstraintViolationError, + 'violates exclusivity constraint' + ): + await self.con.execute(""" + update UniqueName + filter .translated_labels_tgt.text = 'x' + set { translated_labels_tgt := + .translated_labels_tgt { @lang := '!' } + } + """) + + await self.con.execute(""" + update UniqueName + filter .translated_labels_tgt.text = 'x' + set { translated_labels_tgt := + .translated_labels_tgt { @lang := @lang } + } + """) + class TestConstraintsSchemaMigration(tb.QueryTestCase):