From 91fa52638fa434a0d6cecba3ed97c312f18b076a Mon Sep 17 00:00:00 2001
From: James Clarke <james@edgedb.com>
Date: Fri, 7 Feb 2025 18:11:12 +0000
Subject: [PATCH] Fix querybuilder generated query when using unpathed scoped
 expr (#1192)

Fixes #808

When walking the expr tree the scoped copy of a select/update expr
passed to the shape should have been treated as an opaque reference and
not walked, since it will get walked again by the enclosing
select/update.

Currently we are only checking this in path expressions (eg.
`movie.title`) and not when the scoped expr appears anywhere else (eg.
`movie.is(e.Movie)`). And since the expr is getting walked multiple
times, it's messing up the ref counting, and exprs are getting hoisted
when they shouldn't, causing the broken queries in #808.
---
 integration-tests/lts/select.test.ts     | 27 ++++++++++++++++++++++++
 packages/generate/src/syntax/toEdgeQL.ts | 22 +++++++++----------
 2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/integration-tests/lts/select.test.ts b/integration-tests/lts/select.test.ts
index 664424fb3..c096f3afd 100644
--- a/integration-tests/lts/select.test.ts
+++ b/integration-tests/lts/select.test.ts
@@ -1712,4 +1712,31 @@ SELECT __scope_0_defaultPerson {
     const result = await e.select(e.json(-0)).run(client);
     assert.equal(result, 0);
   });
+
+  test("select using scoped expr directly (not a field/path on that scoped expr)", async () => {
+    const query = e.select(
+      e.select(e.Movie, (movie) => ({
+        filter: e.op(movie.title, "=", "Dune"),
+      })),
+      (movie) => ({
+        comp: movie.is(e.Movie),
+      }),
+    );
+    await query.run(client);
+
+    const user = e.select(e.User, () => ({
+      filter_single: { id: "4d0f90b1-de94-4c79-ba56-3e0acdfbd06d" },
+    }));
+    const movies = e.select(user.favourite_movies, (movie) => ({
+      filter: e.op(movie.title, "=", user.username),
+    }));
+    const query2 = e.with(
+      [user, movies],
+      e.select(movies, (movie) => ({
+        isFav: e.op(movie, "in", user.favourite_movies),
+      })),
+    );
+
+    await query2.run(client);
+  });
 });
diff --git a/packages/generate/src/syntax/toEdgeQL.ts b/packages/generate/src/syntax/toEdgeQL.ts
index e1b1cb343..8be7d948f 100644
--- a/packages/generate/src/syntax/toEdgeQL.ts
+++ b/packages/generate/src/syntax/toEdgeQL.ts
@@ -397,6 +397,13 @@ function walkExprTree(
 
   const expr = _expr as SomeExpression;
 
+  if ((expr as any).__scopedFrom__ != null) {
+    // If expr is marked as being a scoped copy of another expr, treat it as
+    // an opaque reference and don't walk it. The enclosing select/update that
+    // owns the scope will walk the actual unscoped expr.
+    return [expr];
+  }
+
   function walkShape(shape: object) {
     for (let param of Object.values(shape)) {
       if (param.__kind__ === ExpressionKind.PolyShapeElement) {
@@ -473,16 +480,9 @@ function walkExprTree(
     case ExpressionKind.PathLeaf:
     case ExpressionKind.PathNode:
       if (expr.__parent__) {
-        if ((expr.__parent__.type as any).__scopedFrom__) {
-          // if parent is scoped expr then don't walk expr
-          // since it will already be walked by enclosing select/update
-
-          childExprs.push(expr.__parent__.type as any);
-        } else {
-          childExprs.push(
-            ...walkExprTree(expr.__parent__.type, parentScope, ctx),
-          );
-        }
+        childExprs.push(
+          ...walkExprTree(expr.__parent__.type, parentScope, ctx),
+        );
 
         if (
           // is link prop
@@ -1226,7 +1226,7 @@ ${indent(groupStatement.join("\n"), 4)}
         );
     }
   } else if (expr.__kind__ === ExpressionKind.TypeIntersection) {
-    return `${renderEdgeQL(expr.__expr__, ctx)}[IS ${
+    return `${renderEdgeQL(expr.__expr__, ctx, false)}[IS ${
       expr.__element__.__name__
     }]`;
   } else if (expr.__kind__ === ExpressionKind.For) {