Repository: geldata/gel
Pull request: For query_single, only check that the *last* query in a script is single
Previously we errored if *any* were, which is needless.

Fixes #4311.

Problem statement:
cardinality missmatch
<!-- Please search existing issues to avoid creating duplicates. -->
- EdgeDB Version: 2.1
- OS Version: ubuntu 22.04

If I send `command.edgeql` (below) as the command to parse with expected cardinality `MANY` it returns cardinality `AT_MOST_ONE`. However if I send the same command to parse with expected cardinality `AT_MOST_ONE` then I get `edgedb.ResultCardinalityMismatchError: the query has cardinality MANY which does not match the expected cardinality ONE`. It seems like the cardinality result from parse with expected cardinality `MANY` should be the actual cardinality and not change with a different expected cardinality value.

```edgeql
# command.edgeql

with
    a := assert_exists((
        select `Set`
        filter .id = <uuid>$set_id and .workout.id = <uuid>$workout_id
    )),
update a
set {
    tmp_index := a.index,
    index := count(a.workout.sets),
};


with
    a := (select `Set` filter .id = <uuid>$set_id),
    to_shift := (
        select `Set`
        filter
            .index > a.tmp_index and
            .workout = a.workout
    ),
for s in to_shift union (
    update s set {
        index := s.index - 1,
    }
);


delete `Set` filter .id = <uuid>$set_id;


select {
    workout := (
        select Workout {
            id,
            date,
            sets: {
                id,
                exercise: { id, label },
                workout: { id },
                reps,
                lbs,
                complete,
            } order by .index
        } filter .id = <uuid>$workout_id
    ),
    exercises := (
        select Exercise {
            id,
            label,
        } order by .label
    ),
};
```

```esdl
# dbschema/default.esdl

module default {
    type Workout {
        multi link sets := .<workout[is `Set`];
        required property date -> cal::local_date {
            constraint exclusive;
        }
    }

    type Exercise {
        required property label -> str {
            constraint exclusive on (str_trim(str_lower(__subject__)));
        };
    }

    type `Set` {
        required link workout -> Workout {
            on target delete delete source;
        };
        required link exercise -> Exercise;

        required property index -> int64;
        property tmp_index -> int64;

        required property reps -> int64 {
            constraint min_value(1);
        };
        required property lbs -> float64 {
            constraint min_value(0);
        };
        required property complete -> bool {
            default := false;
        }

        constraint exclusive on ((.index, .workout))
    }
}
```

Hints from issue discussion:
Also, the error reports a different expected cardinality than the one that was used in the parse message.

Commit message: For query_single, only check that the *last* query in a script is single

Previously we errored if *any* were, which is needless.

Fixes #4311.

Patch to review:
diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py
index 1f5470b1cc2..8448e7eec74 100644
--- a/edb/server/compiler/compiler.py
+++ b/edb/server/compiler/compiler.py
@@ -625,10 +625,6 @@ def _compile_ql_query(
             options=self._get_compile_options(ctx),
         )
 
-        if ir.cardinality.is_multi() and ctx.expected_cardinality_one:
-            raise errors.ResultCardinalityMismatchError(
-                f'the query has cardinality {ir.cardinality.name} '
-                f'which does not match the expected cardinality ONE')
         result_cardinality = enums.cardinality_from_ir_value(ir.cardinality)
 
         sql_text, argmap = pg_compiler.compile_ir_to_sql(
@@ -2012,6 +2008,14 @@ def _try_compile(
                 raise errors.InternalServerError(
                     f'QueryUnit {unit!r} is cacheable but has config/aliases')
 
+            multi_card = unit.cardinality in (
+                enums.Cardinality.MANY, enums.Cardinality.AT_LEAST_ONE,
+            )
+            if multi_card and ctx.expected_cardinality_one:
+                raise errors.ResultCardinalityMismatchError(
+                    f'the query has cardinality {unit.cardinality.name} '
+                    f'which does not match the expected cardinality ONE')
+
             if not na_cardinality and (
                     len(unit.sql) > 1 or
                     unit.tx_commit or
diff --git a/tests/test_server_proto.py b/tests/test_server_proto.py
index 1dbf47c31c6..0748b379843 100644
--- a/tests/test_server_proto.py
+++ b/tests/test_server_proto.py
@@ -3058,6 +3058,15 @@ async def test_fetch_elements(self):
         ''')
         self.assertEqual(result, ['"test1"', '"test2"'])
 
+    async def test_query_single_script(self):
+        # query single should work even if earlier parts of the script
+        # are multisets
+        result = await self.con.query_single('''
+            select {1, 2};
+            select 1;
+        ''')
+        self.assertEqual(result, 1)
+
 
 class TestServerProtoConcurrentDDL(tb.DDLTestCase):
 


Code review comments:
diff_hunk: @@ -2012,6 +2008,14 @@ def _try_compile(
                 raise errors.InternalServerError(
                     f'QueryUnit {unit!r} is cacheable but has config/aliases')
 
+            multi_card = unit.cardinality in (
+                enums.Cardinality.MANY, enums.Cardinality.AT_LEAST_ONE,
+            )
+            if multi_card and ctx.expected_cardinality_one:
path: edb/server/compiler/compiler.py
text: @user1:
Took me a while to figure out that I need to look at: https://github.com/edgedb/edgedb/blob/ad92c48d26f9052371dfc48d31e414f592e16485/edb/server/compiler/compiler.py#L1854-L1855

so that non-trailing query units have the cardinality `NO_RESULT`. Would be more readable if we can just check `rv[-1]`, or add a comment?

@author:
Oh, yeah, I actually meant to be doing this logic on `rv` itself!