Repository: geldata/gel
Pull request: Consider object exclusive constraints in cardinality inference
This makes object exclusive constraints more consistent with pointer
ones and allows easily filtering on compound exclusive constraints.

We do this by looking at tuple values in addition to directly pointer
references.

Fixes #2097. Lays the groundwork for tuple ON CONFLICT.

Problem statement:
Possibly inconsistent cardinality determination
With the following schema:

```
type Person {
    required property name -> str;
    constraint exclusive on ((.name,));
}
```

If I run:

```python
query_one_json("""SELECT Person FILTER .name = 'test'""")
```

I get:

```
edgedb.errors.InterfaceError: query cannot be executed with query_one_json() as it returns a multiset
```

But if I do:

```python
query_one_json("""SELECT Person FILTER .id = <uuid>'...'""")
```

I get a singular result, as expected.

I would have thought that since `constraint exclusive on ((.name,))` creates an index, the cardinality would have been the same as filtering on `id`. Please let me know if I am misunderstanding or doing something wrong.

Edit: upon further inspection, this does not seem to be a problem with the Python driver itself (so sorry for posting here), since I also encountered similar behavior when using `UNLESS CONFLICT ON`:

```
INSERT Person { name := 'test' }
UNLESS CONFLICT ON .name
ELSE (...)
```

Fails as well. That lead me to try:

```
type Person {
    required property name -> str { constraint exclusive };
}
```

Which, does resolve both problems, but isn't equivalent for the case of:

```
type Person {
    required property first_name -> str;
    required property last_name -> str;
    constraint exclusive on ((.first_name, .last_name,));
}
```

Hints from issue discussion:
@elprans
This is a server-side inference issue.  @msullivan, we need to take object-level exclusive constraints into account when inferring cardinalities.

Commit message: Consider object exclusive constraints in cardinality inference

This makes object exclusive constraints more consistent with pointer
ones and allows easily filtering on compound exclusive constraints.

We do this by looking at tuple values in addition to directly pointer
references.

Fixes #2097. Lays the groundwork for tuple ON CONFLICT.

Patch to review:
diff --git a/edb/edgeql/compiler/inference/cardinality.py b/edb/edgeql/compiler/inference/cardinality.py
index d47129023d6..a257085b90c 100644
--- a/edb/edgeql/compiler/inference/cardinality.py
+++ b/edb/edgeql/compiler/inference/cardinality.py
@@ -36,8 +36,11 @@
 from edb.edgeql import qltypes
 
 from edb.schema import name as sn
+from edb.schema import types as s_types
 from edb.schema import objtypes as s_objtypes
 from edb.schema import pointers as s_pointers
+from edb.schema import constraints as s_constraints
+from edb.schema import expr as s_expr
 
 from edb.ir import ast as irast
 from edb.ir import utils as irutils
@@ -48,6 +51,7 @@
 from . import context as inference_context
 
 from .. import context
+from .. import options
 
 
 AT_MOST_ONE = qltypes.Cardinality.AT_MOST_ONE
@@ -932,6 +936,82 @@ def extract_filters(
     return None
 
 
+def _get_object_constraint_pointers(
+    typ: s_objtypes.ObjectType,
+    constr: s_constraints.Constraint,
+    env: context.Environment,
+) -> Optional[Sequence[s_pointers.PointerLike]]:
+    subjectexpr = constr.get_subjectexpr(env.schema)
+    if not subjectexpr:
+        return None
+
+    # would it be worth it to cache this?
+    subjectexpr = s_expr.Expression.compiled(
+        subjectexpr,
+        schema=env.schema,
+        options=options.CompilerOptions(
+            anchors={'__subject__': typ},
+            path_prefix_anchor='__subject__',
+        ),
+    )
+    ptrrefs = irutils.extract_constraint_style_ptrs(
+        irutils.unwrap_set(subjectexpr.ir_statement.expr))
+    if ptrrefs is None:
+        return None
+
+    parts = []
+    for rptr in ptrrefs:
+        env.schema, ptrcls = typeutils.ptrcls_from_ptrref(
+            rptr.ptrref, schema=env.schema)
+        parts.append(ptrcls)
+
+    return parts
+
+
+def get_object_exclusive_constraints(
+    typ: s_types.Type,
+    ptr_set: Set[s_pointers.Pointer],
+    env: context.Environment,
+) -> Sequence[Tuple[
+    s_constraints.Constraint, Sequence[s_pointers.PointerLike],
+]]:
+    """Collect any exclusive object constraints that could apply.
+
+    Object constraints that are just a single pointer reference or a
+    tuple of pointer references can be used for cardinality purposes.
+
+    Returns a sequence of (constraint, pointer sequence) pair,
+    where each pointer sequence lists all of the component pointers in
+    the exclusive object constraint.
+    """
+
+    if not isinstance(typ, s_objtypes.ObjectType):
+        return ()
+
+    schema = env.schema
+    exclusive = schema.get('std::exclusive', type=s_constraints.Constraint)
+
+    cnstrs = []
+    typ = typ.get_nearest_non_derived_parent(schema)
+    for constr in typ.get_constraints(schema).objects(schema):
+        if (
+            constr.issubclass(schema, exclusive)
+            and (subjectexpr := constr.get_subjectexpr(schema))
+        ):
+            # as an optimization, make sure that all the referenced values
+            # appear in the query so we don't need to compile the subjectexpr
+            # if they don't
+            if subjectexpr.refs is not None and not all(
+                ref in ptr_set for ref in subjectexpr.refs.objects(schema)
+            ):
+                continue
+
+            if res := _get_object_constraint_pointers(typ, constr, env):
+                cnstrs.append((constr, res))
+
+    return cnstrs
+
+
 def _analyse_filter_clause(
     result_set: irast.Set,
     result_card: qltypes.Cardinality,
@@ -945,12 +1025,25 @@ def _analyse_filter_clause(
         result_set, filter_clause, scope_tree, ctx)
 
     if filtered_ptrs:
+        ptr_set = set()
+        # First look at each referenced pointer and see if it has
+        # an exclusive constraint.
         for ptr, _ in filtered_ptrs:
+            ptr = ptr.get_nearest_non_derived_parent(schema)
+            ptr_set.add(ptr)
             if ptr.is_exclusive(schema):
                 # Bingo, got an equality filter on a link with a
                 # unique constraint
                 return AT_MOST_ONE
 
+        # Then look at all the object exclusive constraints
+        result_stype = ctx.env.set_types[result_set]
+        obj_exclusives = get_object_exclusive_constraints(
+            result_stype, ptr_set, ctx.env)
+        for _, exc_ptrs in obj_exclusives:
+            if all(exc_ptr in ptr_set for exc_ptr in exc_ptrs):
+                return AT_MOST_ONE
+
     return result_card
 
 
diff --git a/edb/ir/typeutils.py b/edb/ir/typeutils.py
index a70fab13bd1..22bcb0e2973 100644
--- a/edb/ir/typeutils.py
+++ b/edb/ir/typeutils.py
@@ -665,7 +665,23 @@ def ptrref_from_ptrcls(  # NoQA: F811
     return ptrref
 
 
+@overload
 def ptrcls_from_ptrref(
+    ptrref: irast.PointerRef, *,
+    schema: s_schema.Schema,
+) -> Tuple[s_schema.Schema, s_pointers.Pointer]:
+    ...
+
+
+@overload
+def ptrcls_from_ptrref(  # NoQA: F811
+    ptrref: irast.BasePointerRef, *,
+    schema: s_schema.Schema,
+) -> Tuple[s_schema.Schema, s_pointers.PointerLike]:
+    ...
+
+
+def ptrcls_from_ptrref(  # NoQA: F811
     ptrref: irast.BasePointerRef, *,
     schema: s_schema.Schema,
 ) -> Tuple[s_schema.Schema, s_pointers.PointerLike]:
diff --git a/edb/ir/utils.py b/edb/ir/utils.py
index b5e225db7d4..edc5fdf8f5b 100644
--- a/edb/ir/utils.py
+++ b/edb/ir/utils.py
@@ -272,6 +272,32 @@ def collapse_type_intersection(
     return source, result
 
 
+def extract_constraint_style_ptrs(
+        ir: irast.Set) -> Optional[List[irast.Pointer]]:
+    """Extract ptrrefs from a simple "constraint-style" expression.
+
+    This means that the expression can be a simple pointer reference
+    or a tuple of "constraint-style" expressions.
+    """
+    parts = []
+
+    def _extract(ir: irast.Set) -> bool:
+        if ir.rptr is not None:
+            parts.append(ir.rptr)
+            return True
+        elif isinstance(ir.expr, irast.Tuple):
+            return all(
+                _extract(elem.val) for elem in ir.expr.elements
+            )
+        else:
+            return False
+
+    if _extract(ir):
+        return parts
+    else:
+        return None
+
+
 def get_nearest_dml_stmt(ir_set: irast.Set) -> Optional[irast.MutatingStmt]:
     """For a given *ir_set* representing a Path, return the nearest path
        step that is a DML expression.
diff --git a/edb/schema/expr.py b/edb/schema/expr.py
index 1549d815ac8..e7cc5a7b74b 100644
--- a/edb/schema/expr.py
+++ b/edb/schema/expr.py
@@ -261,7 +261,7 @@ def schema_refs_from_data(
         return so.ObjectCollection.schema_refs_from_data(data[1])
 
     @property
-    def _ir_statement(self) -> irast_.Statement:
+    def ir_statement(self) -> irast_.Statement:
         """Assert this expr is a compiled EdgeQL statement and return its IR"""
         from edb.ir import ast as irast_
 
@@ -275,15 +275,15 @@ def _ir_statement(self) -> irast_.Statement:
 
     @property
     def stype(self) -> s_types.Type:
-        return self._ir_statement.stype
+        return self.ir_statement.stype
 
     @property
     def cardinality(self) -> qltypes.Cardinality:
-        return self._ir_statement.cardinality
+        return self.ir_statement.cardinality
 
     @property
     def schema(self) -> s_schema.Schema:
-        return self._ir_statement.schema
+        return self.ir_statement.schema
 
 
 class ExpressionShell(so.Shell):
diff --git a/tests/schemas/cards_ir_inference.esdl b/tests/schemas/cards_ir_inference.esdl
index a46aff9eb1a..726c1658627 100644
--- a/tests/schemas/cards_ir_inference.esdl
+++ b/tests/schemas/cards_ir_inference.esdl
@@ -102,3 +102,24 @@ type Report extending Named {
         property note -> str;
     }
 }
+
+
+abstract type BadlyNamed {
+    property first -> str;
+    property last -> str;
+    delegated constraint exclusive on ((.first, .last));
+}
+
+
+type Person extending BadlyNamed {
+    # these constraints don't really make sense but that's fine.
+    property email -> str;
+    constraint exclusive on (.email);
+    property p -> int64;
+    property q -> int64;
+    constraint exclusive on (.p * .q);
+    constraint exclusive on (((.p, __subject__.q), __subject__.first));
+
+    link card -> Card;
+    constraint exclusive on ((.p, .card));
+}
diff --git a/tests/test_edgeql_ir_card_inference.py b/tests/test_edgeql_ir_card_inference.py
index 682f95c5043..47443323004 100644
--- a/tests/test_edgeql_ir_card_inference.py
+++ b/tests/test_edgeql_ir_card_inference.py
@@ -556,3 +556,69 @@ def test_edgeql_ir_card_inference_53(self):
 % OK %
         ONE
         """
+
+    # some tests of object constraints
+    def test_edgeql_ir_card_inference_54(self):
+        """
+        WITH MODULE test
+        SELECT Person FILTER .first = "Phil" AND .last = "Emarg"
+% OK %
+        AT_MOST_ONE
+        """
+
+    def test_edgeql_ir_card_inference_55(self):
+        """
+        WITH MODULE test
+        SELECT Person FILTER .first = "Phil"
+% OK %
+        MANY
+        """
+
+    def test_edgeql_ir_card_inference_56(self):
+        """
+        WITH MODULE test
+        SELECT Person FILTER .email = "test@example.com"
+% OK %
+        AT_MOST_ONE
+        """
+
+    def test_edgeql_ir_card_inference_57(self):
+        """
+        WITH MODULE test
+        SELECT Person FILTER .p = 7 AND .q = 3
+% OK %
+        MANY
+        """
+
+    def test_edgeql_ir_card_inference_58(self):
+        """
+        WITH MODULE test
+        SELECT Person FILTER .last = "Hatch" AND .first = "Madeline"
+% OK %
+        AT_MOST_ONE
+        """
+
+    def test_edgeql_ir_card_inference_59(self):
+        """
+        WITH MODULE test
+        SELECT Person FILTER .p = 7 AND .q = 3 AND .first = "???"
+% OK %
+        AT_MOST_ONE
+        """
+
+    def test_edgeql_ir_card_inference_60(self):
+        """
+        WITH MODULE test
+        SELECT Person
+        FILTER .p = 12 AND .card = (SELECT Card FILTER .name = 'Imp')
+% OK %
+        AT_MOST_ONE
+        """
+
+    def test_edgeql_ir_card_inference_61(self):
+        """
+        WITH MODULE test
+        SELECT Person FILTER .first = "Phil" OR .last = "Emarg"
+% OK %
+        MANY
+        """


Code review comments:
diff_hunk: @@ -945,12 +1025,25 @@ def _analyse_filter_clause(
         result_set, filter_clause, scope_tree, ctx)
 
     if filtered_ptrs:
+        ptr_set = set()
+        # First look at each referenced pointer and see if it has
+        # an exclusive constraint.
         for ptr, _ in filtered_ptrs:
+            ptr = ptr.get_nearest_non_derived_parent(schema)
+            ptr_set.add(ptr)
             if ptr.is_exclusive(schema):
                 # Bingo, got an equality filter on a link with a
                 # unique constraint
                 return AT_MOST_ONE
 
+        # Then look at all the object exclusive constraints
+        result_stype = ctx.env.set_types[result_set]
+        obj_exclusives = get_object_exclusive_constraints(
+            result_stype, ptr_set, ctx.env)
+        for _, exc_ptrs in obj_exclusives:
+            if all(exc_ptr in ptr_set for exc_ptr in exc_ptrs):
path: edb/edgeql/compiler/inference/cardinality.py
text: @user1:
Perhaps `if not (exc_ptrs - ptr_set)`?