Skip to content

Commit 9780d84

Browse files
Nguyễn Phúc Lươngclaude
andcommitted
fix(compiler): scope-aware column resolution in subqueries (#4251)
When resolving an unqualified column reference inside a subquery, sqlc's analyzer matched against every table in scope across the whole query, producing a spurious "column reference ... is ambiguous" error for queries that real PostgreSQL resolves unambiguously via lexical scope. Example from the issue: SELECT * FROM t1 WHERE id IN ( SELECT t1_id FROM t2 WHERE id = $1 ); Real Postgres binds the outer "id" to t1.id and the inner "id" to t2.id by lexical scope. sqlc was flattening every FROM-clause RangeVar in the whole query into one search list and triggering "ambiguous" on the inner id. Two small changes: * internal/compiler/find_params.go — when paramSearch.Visit enters a SelectStmt whose FROM clause has exactly one RangeVar, capture it as the current scope (paramSearch.rangeVar). The walker propagates this through the returned visitor, so ParamRefs encountered in the same SelectStmt's WHERE/GROUP/etc. inherit the inner scope. The exactly-one guard ensures we don't silently pick a winner when the FROM clause is genuinely multi-table at the same level — those cases must keep iterating every table so true ambiguity (e.g. "SELECT t1.id FROM t1, t2 WHERE id = $1") still errors. * internal/compiler/resolve.go — when resolving an unqualified column, if no alias is given but ref.rv points to an in-scope table that actually contains the column, narrow the search to that table only. If the column is absent from the inner table, fall back to the full search list so correlated-subquery references to an outer column continue to work. Tests: * internal/compiler/resolve_test.go — new unit test covering narrowToInnermostScope across nil-rv, column-in-inner-scope (narrow), column-absent (fall back), and unknown-table (fall back). * internal/endtoend/testdata/subquery_scope_4251/ — end-to-end fixture reproducing the exact query from the issue under postgresql/pgx/v5; generated code asserted against the golden files. Verified locally: * /tmp/repro4251 generate now returns EXIT=0 (was EXIT=1 with 'relation "id" ambiguous'). * SELECT t1.id FROM t1, t2 WHERE id = $1 still correctly errors with 'ambiguous'. * Correlated subquery (SELECT SUM(total) FROM orders WHERE customer_id = c.id) still resolves cleanly. * go test ./internal/compiler/... ./internal/sql/... ./internal/engine/... passes; gofmt -l and go vet clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a3b0cfd commit 9780d84

9 files changed

Lines changed: 257 additions & 0 deletions

File tree

internal/compiler/find_params.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,18 @@ func (p paramSearch) Visit(node ast.Node) astutils.Visitor {
146146
if n.LimitOffset != nil {
147147
p.limitOffset = n.LimitOffset
148148
}
149+
// Capture this SELECT's FROM-clause scope so that ParamRefs encountered
150+
// while walking deeper into this subtree (WHERE, GROUP BY, etc.) can be
151+
// resolved against the innermost in-scope table. Only narrow when the
152+
// FROM clause is unambiguous on its own — a single RangeVar with no
153+
// JOINs or subselects mixed in. Otherwise leave the scope alone so the
154+
// resolver still iterates every in-scope table and detects genuine
155+
// ambiguity (e.g. SELECT id FROM t1, t2 WHERE id = $1). See issue #4251.
156+
if n.FromClause != nil && len(n.FromClause.Items) == 1 {
157+
if rv, ok := n.FromClause.Items[0].(*ast.RangeVar); ok && rv != nil && rv.Relname != nil {
158+
p.rangeVar = rv
159+
}
160+
}
149161

150162
case *ast.TypeCast:
151163
p.parent = node

internal/compiler/resolve.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,8 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar,
215215
}
216216
}
217217
}
218+
} else if scoped := narrowToInnermostScope(tables, typeMap, c.DefaultSchema, ref.rv, key); scoped != nil {
219+
search = scoped
218220
}
219221

220222
var found int
@@ -581,6 +583,8 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar,
581583
}
582584
}
583585
}
586+
} else if scoped := narrowToInnermostScope(tables, typeMap, c.DefaultSchema, ref.rv, key); scoped != nil {
587+
search = scoped
584588
}
585589

586590
for _, table := range search {
@@ -636,3 +640,48 @@ func (comp *Compiler) resolveCatalogRefs(qc *QueryCatalog, rvs []*ast.RangeVar,
636640
}
637641
return a, nil
638642
}
643+
644+
// narrowToInnermostScope returns a single-element search slice when the
645+
// parameter reference (ref.rv) points to a known table that actually
646+
// contains the column being resolved. It implements the lexical-scope rule
647+
// real PostgreSQL applies inside subqueries: an unqualified column reference
648+
// is bound to the innermost FROM-clause table that defines it. If the
649+
// innermost scope is unknown (rv nil) or the column is not present there
650+
// (correlated-subquery referring to an outer column), it returns nil so the
651+
// caller falls back to the full table list. See issue #4251.
652+
func narrowToInnermostScope(
653+
tables []*ast.TableName,
654+
typeMap map[string]map[string]map[string]*catalog.Column,
655+
defaultSchema string,
656+
rv *ast.RangeVar,
657+
column string,
658+
) []*ast.TableName {
659+
if rv == nil || rv.Relname == nil {
660+
return nil
661+
}
662+
innerName := *rv.Relname
663+
innerSchema := ""
664+
if rv.Schemaname != nil {
665+
innerSchema = *rv.Schemaname
666+
}
667+
lookupSchema := innerSchema
668+
if lookupSchema == "" {
669+
lookupSchema = defaultSchema
670+
}
671+
// Only narrow if the column actually exists in the innermost scope.
672+
// Falling back to the full search preserves correlated-subquery
673+
// behavior (e.g. an inner WHERE referring to an outer column).
674+
if _, ok := typeMap[lookupSchema][innerName][column]; !ok {
675+
return nil
676+
}
677+
for _, t := range tables {
678+
tSchema := t.Schema
679+
if tSchema == "" {
680+
tSchema = defaultSchema
681+
}
682+
if t.Name == innerName && tSchema == lookupSchema {
683+
return []*ast.TableName{t}
684+
}
685+
}
686+
return nil
687+
}

internal/compiler/resolve_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package compiler
2+
3+
import (
4+
"testing"
5+
6+
"github.com/sqlc-dev/sqlc/internal/sql/ast"
7+
"github.com/sqlc-dev/sqlc/internal/sql/catalog"
8+
)
9+
10+
// TestNarrowToInnermostScope covers the lexical-scope rule used by the
11+
// resolver to disambiguate column references that live inside subqueries.
12+
// See issue #4251.
13+
func TestNarrowToInnermostScope(t *testing.T) {
14+
t.Parallel()
15+
16+
str := func(s string) *string { return &s }
17+
tables := []*ast.TableName{
18+
{Name: "t1"},
19+
{Name: "t2"},
20+
}
21+
typeMap := map[string]map[string]map[string]*catalog.Column{
22+
"public": {
23+
"t1": {"id": {Name: "id"}},
24+
"t2": {"id": {Name: "id"}, "t1_id": {Name: "t1_id"}},
25+
},
26+
}
27+
28+
t.Run("nil_rv_returns_nil", func(t *testing.T) {
29+
got := narrowToInnermostScope(tables, typeMap, "public", nil, "id")
30+
if got != nil {
31+
t.Fatalf("expected nil (no narrowing) got %v", got)
32+
}
33+
})
34+
35+
t.Run("nil_relname_returns_nil", func(t *testing.T) {
36+
got := narrowToInnermostScope(tables, typeMap, "public", &ast.RangeVar{}, "id")
37+
if got != nil {
38+
t.Fatalf("expected nil (no narrowing) got %v", got)
39+
}
40+
})
41+
42+
t.Run("column_in_inner_scope_narrows_to_inner_table", func(t *testing.T) {
43+
// The repro shape: ParamRef in inner SELECT (rv=t2) resolving column "id".
44+
// id exists in t2 -> narrow search to [t2] so the outer t1.id doesn't
45+
// trigger a spurious "ambiguous" error.
46+
rv := &ast.RangeVar{Relname: str("t2")}
47+
got := narrowToInnermostScope(tables, typeMap, "public", rv, "id")
48+
if len(got) != 1 || got[0].Name != "t2" {
49+
t.Fatalf("expected narrow to [t2], got %v", got)
50+
}
51+
})
52+
53+
t.Run("column_absent_from_inner_falls_back_to_full_scope", func(t *testing.T) {
54+
// Correlated-subquery shape: inner SELECT (rv=t2) references an outer
55+
// column not present in t2. Returning nil tells the caller to keep the
56+
// full tables list, which lets the outer-scope match win.
57+
rv := &ast.RangeVar{Relname: str("t2")}
58+
got := narrowToInnermostScope(tables, typeMap, "public", rv, "not_a_t2_column")
59+
if got != nil {
60+
t.Fatalf("expected nil (fall back to all tables), got %v", got)
61+
}
62+
})
63+
64+
t.Run("rv_points_to_unknown_table_falls_back", func(t *testing.T) {
65+
rv := &ast.RangeVar{Relname: str("nonexistent")}
66+
got := narrowToInnermostScope(tables, typeMap, "public", rv, "id")
67+
if got != nil {
68+
t.Fatalf("expected nil (fall back), got %v", got)
69+
}
70+
})
71+
}

internal/endtoend/testdata/subquery_scope_4251/postgresql/pgx/v5/go/db.go

Lines changed: 32 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/subquery_scope_4251/postgresql/pgx/v5/go/models.go

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/endtoend/testdata/subquery_scope_4251/postgresql/pgx/v5/go/query.sql.go

Lines changed: 44 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-- name: GetT1FromT2 :many
2+
-- Unqualified `id` inside the subquery must bind to t2.id (innermost
3+
-- FROM-clause scope), not be flagged as ambiguous against t1.id. See
4+
-- issue #4251.
5+
SELECT id FROM t1
6+
WHERE id IN (
7+
SELECT t1_id
8+
FROM t2
9+
WHERE id = $1
10+
);
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
CREATE TABLE t1 (
2+
id UUID PRIMARY KEY
3+
);
4+
5+
CREATE TABLE t2 (
6+
id UUID,
7+
t1_id UUID REFERENCES t1(id) ON DELETE CASCADE
8+
);
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"version": "1",
3+
"packages": [
4+
{
5+
"path": "go",
6+
"engine": "postgresql",
7+
"sql_package": "pgx/v5",
8+
"name": "querytest",
9+
"schema": "schema.sql",
10+
"queries": "query.sql"
11+
}
12+
]
13+
}

0 commit comments

Comments
 (0)