Repository: geldata/gel
Pull request: Fix broken SET OF/OPTIONAL interactions
The interaction between SET OF and OPTIONAL needs more careful
handling than is done currently.
Basically, the idea is:
1. Merging an optional path with a non-optional one should result in a
   non-optional path, *unless* the non-optional path was fenced.
2. When an path is factored out past an optional parent, it should
   become optional. This rule allows us to avoid marking *all* paths
   appearing in an optional argument as optional, which would result
   in worse codegen.

Applying these rules is complicated by the fact that when we compile
arguments, we don't know whether it is a SET OF or an OPTIONAL. We
solve this by tracking how many non-optional paths have been merged
into a previously optional node and doing a post-hoc fixup of all the
paths encountered in SET OF and OPTIONAL arguments.

Fixes #1640.

Problem statement:
Bug in interaction of OPTIONAL and SET OF
<!-- Please search existing issues to avoid creating duplicates. -->
- EdgeDB Version: 1.0-alpha.4+dev.92.g20b0bd7f
- OS Version: Linux

Steps to Reproduce:

```
victor> CREATE TYPE Foo;
OK: CREATE
victor> SELECT (<str>Foo.id) ?? (<str>count(Foo));
{}
```
The expected result is `{'0'}`, since we expect the `count` of an empty set be called.

Hints from issue discussion:
Another example is `SELECT (<str>Foo.id ?= <str>count(Foo));`, which returns `{}` instead of `{false}`.

There seems to be some trickiness, here, though, since we *do* want those sides to be correlated. If we add a `num` field to `Foo`, we want `SELECT (Foo.num ?= count(Foo));` to only be true for objects where num = 1, I think.
> since we _do_ want those sides to be correlated. If we add a `num` field to `Foo`, we want `SELECT (Foo.num ?= count(Foo));` to only be true for objects where num = 1,

That's correct.
A related issue, albeit in the other direction, that probably needs to be tackled together with this:
```
sully> SELECT (Foo ?= Foo, Foo);
{(true, Object {id: {}})}
```

Commit message: Fix another bug--iterate over paths

Patch to review:
diff --git a/edb/edgeql/compiler/context.py b/edb/edgeql/compiler/context.py
index 68241d81c66..c33149b3ee6 100644
--- a/edb/edgeql/compiler/context.py
+++ b/edb/edgeql/compiler/context.py
@@ -491,6 +491,9 @@ class ContextLevel(compiler.ContextLevel):
     """A set of schema objects for which the shadowing rewrite should be
        disabled."""
 
+    path_log: Optional[List[irast.PathId]]
+    """An optional list of path ids added to the scope tree in this context."""
+
     def __init__(
         self,
         prevlevel: Optional[ContextLevel],
@@ -552,6 +555,7 @@ def __init__(
             self.in_temp_scope = False
             self.tentative_work = []
             self.disable_shadowing = set()
+            self.path_log = None
 
         else:
             self.env = prevlevel.env
@@ -599,6 +603,7 @@ def __init__(
             self.in_temp_scope = prevlevel.in_temp_scope
             self.tentative_work = prevlevel.tentative_work
             self.disable_shadowing = prevlevel.disable_shadowing
+            self.path_log = prevlevel.path_log
 
             if mode == ContextSwitchMode.SUBQUERY:
                 self.anchors = prevlevel.anchors.copy()
diff --git a/edb/edgeql/compiler/expr.py b/edb/edgeql/compiler/expr.py
index 9cddc49892f..ab4f48331fb 100644
--- a/edb/edgeql/compiler/expr.py
+++ b/edb/edgeql/compiler/expr.py
@@ -59,8 +59,7 @@ def compile__Optional(
         dispatch.compile(expr.expr, ctx=ctx),
         ctx=ctx)
 
-    pathctx.register_set_in_scope(result, ctx=ctx)
-    pathctx.mark_path_as_optional(result.path_id, ctx=ctx)
+    pathctx.register_set_in_scope(result, optional=True, ctx=ctx)
 
     return result
 
diff --git a/edb/edgeql/compiler/func.py b/edb/edgeql/compiler/func.py
index 8dcf405c747..144ccd7500b 100644
--- a/edb/edgeql/compiler/func.py
+++ b/edb/edgeql/compiler/func.py
@@ -94,7 +94,7 @@ def compile_FunctionCall(
         ctx.env.options.schema_object_context is s_constr.Constraint
     )
 
-    args, kwargs = compile_call_args(expr, funcname, ctx=ctx)
+    args, kwargs, arg_ctxs = compile_call_args(expr, funcname, ctx=ctx)
     matched = polyres.find_callable(funcs, args=args, kwargs=kwargs, ctx=ctx)
     if not matched:
         raise errors.QueryError(
@@ -140,6 +140,7 @@ def compile_FunctionCall(
     final_args, params_typemods = finalize_args(
         matched_call,
         is_polymorphic=is_polymorphic,
+        arg_ctxs=arg_ctxs,
         ctx=ctx,
     )
 
@@ -238,9 +239,11 @@ def compile_operator(
     fq_op_name = next(iter(opers)).get_shortname(ctx.env.schema)
     conditional_args = CONDITIONAL_OPS.get(fq_op_name)
 
+    arg_ctxs = {}
     args = []
     for ai, qlarg in enumerate(qlargs):
         with ctx.newscope(fenced=True) as fencectx:
+            fencectx.path_log = []
             # We put on a SET OF fence preemptively in case this is
             # a SET OF arg, which we don't know yet due to polymorphic
             # matching.  We will remove it if necessary in `finalize_args()`.
@@ -255,6 +258,8 @@ def compile_operator(
                 setgen.ensure_stmt(arg_ir, ctx=fencectx),
                 ctx=fencectx)
 
+            arg_ctxs[arg_ir] = fencectx
+
         arg_type = inference.infer_type(arg_ir, ctx.env)
         if arg_type is None:
             raise errors.QueryError(
@@ -456,6 +461,7 @@ def compile_operator(
 
     final_args, params_typemods = finalize_args(
         matched_call,
+        arg_ctxs=arg_ctxs,
         actual_typemods=actual_typemods,
         is_polymorphic=is_polymorphic,
         ctx=ctx,
@@ -563,9 +569,11 @@ def validate_recursive_operator(
     return matched
 
 
-def compile_call_arg(arg_ql: qlast.Expr, *,
-                     ctx: context.ContextLevel) -> irast.Set:
+def compile_call_arg(
+        arg_ql: qlast.Expr, *,
+        ctx: context.ContextLevel) -> Tuple[irast.Set, context.ContextLevel]:
     with ctx.new() as argctx:
+        argctx.path_log = []
         # We put on a SET OF fence preemptively in case this is
         # a SET OF arg, which we don't know yet due to polymorphic
         # matching.  We will remove it if necessary in `finalize_args()`.
@@ -576,7 +584,7 @@ def compile_call_arg(arg_ql: qlast.Expr, *,
         return setgen.ensure_set(
             dispatch.compile(arg_ql, ctx=argctx),
             ctx=argctx,
-        )
+        ), argctx
 
 
 def compile_call_args(
@@ -584,13 +592,15 @@ def compile_call_args(
         ctx: context.ContextLevel) \
         -> Tuple[
             List[Tuple[s_types.Type, irast.Set]],
-            Dict[str, Tuple[s_types.Type, irast.Set]]]:
+            Dict[str, Tuple[s_types.Type, irast.Set]],
+            Dict[irast.Set, context.ContextLevel]]:
 
     args = []
     kwargs = {}
+    arg_ctxs = {}
 
     for ai, arg in enumerate(expr.args):
-        arg_ir = compile_call_arg(arg, ctx=ctx)
+        arg_ir, arg_ctx = compile_call_arg(arg, ctx=ctx)
         arg_type = inference.infer_type(arg_ir, ctx.env)
         if arg_type is None:
             raise errors.QueryError(
@@ -598,10 +608,11 @@ def compile_call_args(
                 f'#{ai} of function {funcname}',
                 context=arg.context)
 
+        arg_ctxs[arg_ir] = arg_ctx
         args.append((arg_type, arg_ir))
 
     for aname, arg in expr.kwargs.items():
-        arg_ir = compile_call_arg(arg, ctx=ctx)
+        arg_ir, arg_ctx = compile_call_arg(arg, ctx=ctx)
 
         arg_type = inference.infer_type(arg_ir, ctx.env)
         if arg_type is None:
@@ -610,13 +621,40 @@ def compile_call_args(
                 f'${aname} of function {funcname}',
                 context=arg.context)
 
+        arg_ctxs[arg_ir] = arg_ctx
         kwargs[aname] = (arg_type, arg_ir)
 
-    return args, kwargs
+    return args, kwargs, arg_ctxs
+
+
+def process_path_log(arg_ctx: Optional[context.ContextLevel],
+                     arg_scope: Optional[irast.ScopeTreeNode]) -> None:
+    if arg_ctx and arg_ctx.path_log is not None:
+        # Since we don't know whether arguments are OPTIONAL or SET OF
+        # until after doing polymorphic matching, paths in an OPTIONAL
+        # or SET OF argument could get factored into an existing
+        # optional node, destroying its optionality.
+        #
+        # To fix this, we track all of the paths attached to the tree
+        # while compiling an argument and find and adjust the
+        # optionality of any factored out nodes after the fact.
+        for path_id in arg_ctx.path_log:
+            for prefix in path_id.iter_prefixes(include_ptr=True):
+                assert arg_scope is not None
+                # If the node is still here, nothing to do
+                desc = arg_scope.find_descendant(prefix)
+                if desc:
+                    continue
+                visible = arg_scope.find_visible(prefix)
+                if visible:
+                    if visible.optional_count:
+                        visible.optional_count -= 1
+        arg_ctx.path_log = []
 
 
 def finalize_args(
     bound_call: polyres.BoundCall, *,
+    arg_ctxs: Dict[irast.Set, context.ContextLevel],
     actual_typemods: Sequence[ft.TypeModifier] = (),
     is_polymorphic: bool = False,
     ctx: context.ContextLevel,
@@ -641,26 +679,29 @@ def finalize_args(
 
         typemods.append(param_mod)
 
+        orig_arg = arg
+        arg_ctx = arg_ctxs.get(orig_arg)
+        arg_scope = pathctx.get_set_scope(arg, ctx=ctx)
         if param_mod is not ft.TypeModifier.SET_OF:
-            arg_scope = pathctx.get_set_scope(arg, ctx=ctx)
             param_shortname = param.get_parameter_name(ctx.env.schema)
 
             # Arg was wrapped for scope fencing purposes,
             # but that fence has been removed above, so unwrap it.
-            orig_arg = arg
             arg = irutils.unwrap_set(arg)
 
             if (param_mod is ft.TypeModifier.OPTIONAL or
                     param_shortname in bound_call.null_args):
 
+                process_path_log(arg_ctx, arg_scope)
+
                 if arg_scope is not None:
                     # Due to the construction of relgen, the (unfenced)
                     # subscope is necessary to shield LHS paths from the outer
                     # query to prevent path binding which may break OPTIONAL.
+                    arg_scope.mark_as_optional()
                     branch = arg_scope.unfence()
 
-                pathctx.register_set_in_scope(arg, ctx=ctx)
-                pathctx.mark_path_as_optional(arg.path_id, ctx=ctx)
+                pathctx.register_set_in_scope(arg, optional=True, ctx=ctx)
 
                 if arg_scope is not None:
                     pathctx.assign_set_scope(arg, branch, ctx=ctx)
@@ -670,6 +711,8 @@ def finalize_args(
                 if arg is orig_arg:
                     pathctx.assign_set_scope(arg, None, ctx=ctx)
         else:
+            process_path_log(arg_ctx, arg_scope)
+
             if (is_polymorphic
                     and ctx.expr_exposed
                     and ctx.implicit_limit
@@ -713,4 +756,9 @@ def finalize_args(
 
         args.append(call_arg)
 
+        # If we have any logged paths left over and our enclosing
+        # context is logging paths, propagate them up.
+        if arg_ctx and arg_ctx.path_log and ctx.path_log is not None:
+            ctx.path_log.extend(arg_ctx.path_log)
+
     return args, typemods
diff --git a/edb/edgeql/compiler/pathctx.py b/edb/edgeql/compiler/pathctx.py
index 96f5d7604d5..e76026733d2 100644
--- a/edb/edgeql/compiler/pathctx.py
+++ b/edb/edgeql/compiler/pathctx.py
@@ -84,13 +84,18 @@ def get_expression_path_id(
 def register_set_in_scope(
         ir_set: irast.Set, *,
         path_scope: irast.ScopeTreeNode=None,
+        optional: bool=False,
         fence_points: FrozenSet[irast.PathId]=frozenset(),
         ctx: context.ContextLevel) -> List[irast.ScopeTreeNode]:
     if path_scope is None:
         path_scope = ctx.path_scope
 
+    if ctx.path_log is not None:
+        ctx.path_log.append(ir_set.path_id)
+
     return path_scope.attach_path(
         ir_set.path_id,
+        optional=optional,
         fence_points=fence_points,
         context=ir_set.context,
     )
@@ -128,12 +133,6 @@ def get_set_scope(
         return scope
 
 
-def mark_path_as_optional(
-        path_id: irast.PathId, *,
-        ctx: context.ContextLevel) -> None:
-    ctx.path_scope.mark_as_optional(path_id)
-
-
 def extend_path_id(
     path_id: irast.PathId,
     *,
diff --git a/edb/ir/scopetree.py b/edb/ir/scopetree.py
index d33d4dd6b6b..bec57c6f74f 100644
--- a/edb/ir/scopetree.py
+++ b/edb/ir/scopetree.py
@@ -63,8 +63,13 @@ class ScopeTreeNode:
     factoring_allowlist: Set[pathid.PathId]
     """A list of prefixes that are always allowed to be factored."""
 
-    optional: bool
-    """Whether this node represents an optional path."""
+    optional_count: Optional[int]
+    """Whether this node represents an optional path.
+
+    If None, the node is not an optional path. If 0, it is.
+    If positive, the node *was* an optional path but has had non-optional
+    nodes merged into it, but could become optional again.
+    """
 
     children: Set[ScopeTreeNode]
     """A set of child nodes."""
@@ -84,6 +89,7 @@ def __init__(
         path_id: Optional[pathid.PathId]=None,
         fenced: bool=False,
         unique_id: Optional[int]=None,
+        optional: bool=False,
     ) -> None:
         self.unique_id = unique_id
         self.path_id = path_id
@@ -92,7 +98,7 @@ def __init__(
         self.unnest_fence = False
         self.factoring_fence = False
         self.factoring_allowlist = set()
-        self.optional = False
+        self.optional_count = 0 if optional else None
         self.children = set()
         self.namespaces = set()
         self._parent: Optional[weakref.ReferenceType[ScopeTreeNode]] = None
@@ -105,7 +111,7 @@ def _copy(self, parent: Optional[ScopeTreeNode]) -> ScopeTreeNode:
         cp = self.__class__(
             path_id=self.path_id,
             fenced=self.fenced)
-        cp.optional = self.optional
+        cp.optional_count = self.optional_count
         cp.unnest_fence = self.unnest_fence
         cp.factoring_fence = self.factoring_fence
         cp.namespaces = set(self.namespaces)
@@ -125,10 +131,14 @@ def name(self) -> str:
 
     def _name(self, debug: bool) -> str:
         if self.path_id is None:
-            return f'FENCE' if self.fenced else f'BRANCH'
+            name = f'FENCE' if self.fenced else f'BRANCH'
         else:
-            pid = self.path_id.pformat_internal(debug=debug)
-            return f'{pid}{" [OPT]" if self.optional else ""}'
+            name = self.path_id.pformat_internal(debug=debug)
+        ocount = self.optional_count
+        return (
+            f'{name}{" [OPT]" if self.optional else ""}'
+            f'{" ["+str(ocount)+"]" if ocount else ""}'
+        )
 
     def debugname(self, fuller: bool=False) -> str:
         parts = [f'{self._name(debug=fuller)}']
@@ -143,6 +153,10 @@ def debugname(self, fuller: bool=False) -> str:
         parts.append(f'0x{id(self):0x}')
         return ' '.join(parts)
 
+    @property
+    def optional(self) -> bool:
+        return self.optional_count == 0
+
     @property
     def fence_info(self) -> FenceInfo:
         return FenceInfo(
@@ -344,6 +358,7 @@ def attach_path(
         self,
         path_id: pathid.PathId,
         *,
+        optional: bool=False,
         context: Optional[pctx.ParserContext],
         fence_points: FrozenSet[pathid.PathId] = frozenset(),
     ) -> List[ScopeTreeNode]:
@@ -358,7 +373,8 @@ def attach_path(
                 is_lprop = True
                 continue
 
-            new_child = ScopeTreeNode(path_id=prefix)
+            new_child = ScopeTreeNode(path_id=prefix,
+                                      optional=optional and parent is subtree)
             parent.attach_child(new_child)
 
             # If the path is a link property, or a tuple
@@ -406,6 +422,7 @@ def attach_path(
         return fences
 
     def attach_subtree(self, node: ScopeTreeNode,
+                       was_fenced: bool=False,
                        context: Optional[pctx.ParserContext]=None) -> None:
         """Attach a subtree to this node.
 
@@ -427,6 +444,7 @@ def attach_subtree(self, node: ScopeTreeNode,
         for descendant in node.path_descendants:
             path_id = descendant.path_id.strip_namespace(dns)
             visible, visible_finfo = self.find_visible_ex(path_id)
+            desc_optional = descendant.is_optional_upto(node.parent)
             if visible is not None:
                 if visible_finfo is not None and visible_finfo.factoring_fence:
                     # This node is already present in the surrounding
@@ -440,10 +458,15 @@ def attach_subtree(self, node: ScopeTreeNode,
                     )
 
                 # This path is already present in the tree, discard,
-                # but keep its OPTIONAL status, if any.
+                # but merge its OPTIONAL status, if any.
+                desc_fenced = descendant.fence is not node.fence or was_fenced
                 descendant.remove()
-                if descendant.optional:
-                    visible.optional = True
+                keep_optional = desc_optional or desc_fenced
+                if (
+                    visible.optional_count is not None
+                    and not keep_optional
+                ):
+                    visible.optional_count += 1
 
             elif descendant.parent_fence is node:
                 # Unfenced path.
@@ -472,10 +495,18 @@ def attach_subtree(self, node: ScopeTreeNode,
                     existing, unnest_fence = self.find_unfenced(path_id)
                     if existing is not None:
                         parent_fence = existing.parent_fence
+                    existing_fenced = False
                 else:
+                    existing_fenced = (
+                        existing.parent_fence is not self.fence)
                     parent_fence = self.fence
 
                 if existing is not None and parent_fence is not None:
+                    if existing.is_optional_upto(parent_fence):
+                        existing.mark_as_optional()
+                    if desc_optional:
+                        descendant.mark_as_optional()
+
                     if parent_fence.find_child(path_id) is None:
                         assert existing.path_id is not None
 
@@ -521,7 +552,8 @@ def attach_subtree(self, node: ScopeTreeNode,
                         parent_fence.attach_child(existing)
 
                     # Discard the node from the subtree being attached.
-                    existing.fuse_subtree(descendant)
+                    existing.fuse_subtree(descendant,
+                                          self_fenced=existing_fenced)
 
         for child in tuple(node.children):
             # Attach whatever is remaining in the subtree.
@@ -535,19 +567,27 @@ def attach_subtree(self, node: ScopeTreeNode,
     def fuse_subtree(
         self,
         node: ScopeTreeNode,
+        self_fenced: bool=False,
     ) -> None:
         node.remove()
 
+        if (
+            self.optional_count is not None
+            and not node.optional
+        ):
+            self.optional_count += 1
+        if node.optional and self_fenced:
+            self.mark_as_optional()
+
         if node.path_id is not None:
-            if node.optional:
-                self.optional = True
             subtree = ScopeTreeNode(fenced=True)
+            subtree.optional_count = node.optional_count
             for child in tuple(node.children):
                 subtree.attach_child(child)
         else:
             subtree = node
 
-        self.attach_subtree(subtree)
+        self.attach_subtree(subtree, was_fenced=self_fenced)
 
     def remove_subtree(self, node: ScopeTreeNode) -> None:
         """Remove the given subtree from this node."""
@@ -569,11 +609,9 @@ def remove_descendants(self, path_id: pathid.PathId) -> None:
         for node in matching:
             node.remove()
 
-    def mark_as_optional(self, path_id: pathid.PathId) -> None:
-        """Indicate that *path_id* is used as an OPTIONAL argument."""
-        node = self.find_visible(path_id)
-        if node is not None:
-            node.optional = True
+    def mark_as_optional(self) -> None:
+        """Indicate that this scope is used as an OPTIONAL argument."""
+        self.optional_count = 0
 
     def is_optional(self, path_id: pathid.PathId) -> bool:
         node = self.find_visible(path_id)
@@ -628,7 +666,7 @@ def unfence(self) -> ScopeTreeNode:
         if parent is None:
             raise ValueError('cannot unfence the root node')
 
-        subtree = ScopeTreeNode()
+        subtree = ScopeTreeNode(optional=self.optional)
 
         for child in list(self.children):
             subtree.attach_child(child)
@@ -790,6 +828,14 @@ def find_descendant_and_ns(
 
         return None, frozenset(), None
 
+    def is_optional_upto(self, ancestor: Optional[ScopeTreeNode]) -> bool:
+        nobe: Optional[ScopeTreeNode] = self
+        while nobe and nobe is not ancestor:
+            if nobe.optional_count is not None:
+                return True
+            nobe = nobe.parent
+        return False
+
     def find_unfenced(self, path_id: pathid.PathId) \
             -> Tuple[Optional[ScopeTreeNode], bool]:
         """Find the unfenced node with the given *path_id*."""
diff --git a/edb/pgsql/compiler/pathctx.py b/edb/pgsql/compiler/pathctx.py
index cd5fd94fc81..9662dedba48 100644
--- a/edb/pgsql/compiler/pathctx.py
+++ b/edb/pgsql/compiler/pathctx.py
@@ -772,19 +772,6 @@ def _get_rel_path_output(
     return result
 
 
-def find_path_output(
-        rel: pgast.BaseRelation, path_id: irast.PathId, ref: pgast.BaseExpr, *,
-        env: context.Environment) -> Optional[pgast.OutputVar]:
-    if isinstance(ref, pgast.TupleVarBase):
-        return None
-
-    for key, other_ref in rel.path_namespace.items():
-        if _same_expr(other_ref, ref) and key in rel.path_outputs:
-            return rel.path_outputs.get(key)
-    else:
-        return None
-
-
 def get_path_output(
         rel: pgast.BaseRelation, path_id: irast.PathId, *,
         aspect: str, allow_nullable: bool=True,
@@ -817,7 +804,10 @@ def _get_path_output(
         # reference to the Object itself.
         src_path_id = path_id.src_path()
         assert src_path_id is not None
-        id_output = rel.path_outputs.get((src_path_id, 'value'))
+        id_output = maybe_get_path_output(rel, src_path_id,
+                                          aspect='value',
+                                          allow_nullable=allow_nullable,
+                                          ptr_info=ptr_info, env=env)
         if id_output is not None:
             _put_path_output_var(rel, path_id, aspect, id_output, env=env)
             return id_output
@@ -835,11 +825,6 @@ def _get_path_output(
     else:
         ref = get_path_var(rel, path_id, aspect=aspect, env=env)
 
-    other_output = find_path_output(rel, path_id, ref, env=env)
-    if other_output is not None:
-        _put_path_output_var(rel, path_id, aspect, other_output, env=env)
-        return other_output
-
     if isinstance(ref, pgast.TupleVarBase):
         elements = []
         for el in ref.elements:
@@ -918,11 +903,12 @@ def _get_path_output(
 
 def maybe_get_path_output(
         rel: pgast.BaseRelation, path_id: irast.PathId, *,
-        aspect: str,
+        aspect: str, allow_nullable: bool=True,
         ptr_info: Optional[pg_types.PointerStorageInfo]=None,
         env: context.Environment) -> Optional[pgast.OutputVar]:
     try:
         return get_path_output(rel, path_id=path_id, aspect=aspect,
+                               allow_nullable=allow_nullable,
                                ptr_info=ptr_info, env=env)
     except LookupError:
         return None
diff --git a/tests/test_edgeql_coalesce.py b/tests/test_edgeql_coalesce.py
index 92fdd94faeb..c2830907eaf 100644
--- a/tests/test_edgeql_coalesce.py
+++ b/tests/test_edgeql_coalesce.py
@@ -20,6 +20,7 @@
 import os.path
 
 from edb.testbase import server as tb
+from edb.tools import test
 
 
 class TestEdgeQLCoalesce(tb.QueryTestCase):
@@ -1274,3 +1275,159 @@ async def test_edgeql_coalesce_wrapping_optional(self):
             ''',
             ['a'],
         )
+
+    async def test_edgeql_coalesce_set_of_01(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT <str>Publication.id ?? <str>count(Publication)
+            ''',
+            ['0'],
+        )
+
+    async def test_edgeql_coalesce_set_of_02(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT Publication.title ?? <str>count(Publication)
+            ''',
+            ['0'],
+        )
+
+    async def test_edgeql_coalesce_set_of_03(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT <str>Publication.id ?= <str>count(Publication)
+            ''',
+            [False],
+        )
+
+    async def test_edgeql_coalesce_set_of_04(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT Publication.title ?= <str>count(Publication)
+            ''',
+            [False],
+        )
+
+    async def test_edgeql_coalesce_set_of_05(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT (Publication.title ?? <str>count(Publication))
+                       ?? Publication.title
+            ''',
+            ['0'],
+        )
+
+    async def test_edgeql_coalesce_set_of_06(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT (Publication.title ?= <str>count(Publication),
+                        Publication)
+            ''',
+            [],
+        )
+
+    async def test_edgeql_coalesce_set_of_07(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT (Publication.title ?= '0',
+                        (Publication.title ?? <str>count(Publication)));
+            ''',
+            [[False, '0']],
+        )
+
+    async def test_edgeql_coalesce_set_of_08(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT ("1" if Publication.title ?= "foo" else "2") ++
+                       (Publication.title ?? <str>count(Publication))
+            ''',
+            ['20'],
+        )
+
+    async def test_edgeql_coalesce_set_of_09(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT (Publication.title ?= "Foo", Publication.title ?= "bar")
+            ''',
+            [[False, False]],
+        )
+
+    async def test_edgeql_coalesce_set_of_10(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT (Publication.title++Publication.title ?= "Foo",
+                        Publication.title ?= "bar")
+            ''',
+            [[False, False]],
+        )
+
+    async def test_edgeql_coalesce_set_of_11(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT (Publication.title ?= "", count(Publication))
+            ''',
+            [[False, 0]],
+        )
+
+    async def test_edgeql_coalesce_set_of_12(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT (
+                    Publication ?= Publication,
+                    (Publication.title++Publication.title
+                       ?= Publication.title) ?=
+                    (Publication ?!= Publication)
+                )
+            ''',
+            [[True, False]]
+        )
+
+    async def test_edgeql_coalesce_set_of_13(self):
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT (Publication ?= Publication, Publication)
+            ''',
+            [],
+        )
+
+    async def test_edgeql_coalesce_set_of_nonempty_01(self):
+        await self.con.execute(
+            '''INSERT test::Publication { title := "1" }''')
+        await self.con.execute(
+            '''INSERT test::Publication { title := "asdf" }''')
+
+        await self.assert_query_result(
+            r'''
+                WITH MODULE test
+                SELECT Publication.title ?= <str>count(Publication)
+            ''',
+            [True, False],
+        )
+
+    @test.xfail('''
+        This test produces a bogus NULL object, when it should
+        return an empty set.
+
+        I think it is a issue in relgen.
+    ''')
+    async def test_edgeql_coalesce_self_01(self):
+        # This is busted!
+        await self.assert_query_result(
+            r'''
+                SELECT test::Publication ?? test::Publication
+            ''',
+            [],
+        )
diff --git a/tests/test_edgeql_ir_scopetree.py b/tests/test_edgeql_ir_scopetree.py
index ed93a91d515..ed653f2422d 100644
--- a/tests/test_edgeql_ir_scopetree.py
+++ b/tests/test_edgeql_ir_scopetree.py
@@ -235,7 +235,7 @@ def test_edgeql_ir_scope_tree_10(self):
 
 % OK %
         "FENCE": {
-            "(test::Card)",
+            "(test::Card) [OPT]",
             "(test::Card).>element[IS std::str] [OPT]",
             "FENCE": {
                 "(test::Card).>cost[IS std::int64]"


Code review comments:
diff_hunk: @@ -790,6 +828,14 @@ def find_descendant_and_ns(
 
         return None, frozenset(), None
 
+    def is_optional_upto(self, ancestor: Optional[ScopeTreeNode]) -> bool:
+        nobe: Optional[ScopeTreeNode] = self
path: edb/ir/scopetree.py
text: @user1:
```suggestion
        node: Optional[ScopeTreeNode] = self
```