Skip to content

feat(spansql): support subquery in View Join #13266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rahul2393 merged 3 commits into from
Nov 3, 2025
Merged

feat(spansql): support subquery in View Join #13266

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e500fc
feat: support subquery in View Join
Oct 30, 2025
1104313
chore: add comment and tests since gemini thinks I made a mistake
Oct 30, 2025
a91c626
run go vet locally
Nov 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion spanner/spansql/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4655,8 +4655,26 @@ func (p *parser) parseLit() (Expr, *parseError) {
return BytesLiteral(tok.string), nil
}

// Handle parenthesized expressions.
// Handle parenthesized expressions and scalar subqueries.
// NOTE: The opening "(" has already been consumed by p.next() above (line 4638).
// The parser is now positioned right after the "(", ready to parse the contents.
if tok.value == "(" {
// Look ahead to see if this is a subquery like: (SELECT ...)
// p.sniff("SELECT") peeks at the next token without consuming it.
if p.sniff("SELECT") {

@gemini-code-assist gemini-code-assist Bot Oct 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current lookahead only checks for SELECT to identify a subquery. However, a subquery can also start with a WITH clause. This will cause parsing to fail for valid scalar subqueries that use common table expressions (CTEs).

To fix this, you should also check for the WITH keyword.

For example, a query like SELECT (WITH t AS (SELECT 1) SELECT * FROM t) would fail to parse correctly.

I'd also recommend adding a test case to TestParseExpr to cover subqueries that start with a WITH clause to prevent future regressions.

Suggested change
if p.sniff("SELECT") {
if p.sniff("SELECT") || p.sniff("WITH") {

@anicoll anicoll Oct 30, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WITH is currently not supported and this PR does not aim to resolve it either as it would grow to large.

// Parse the subquery starting from the current position (after the "(")
q, err := p.parseQuery()
if err != nil {
return nil, err
}
if err := p.expect(")"); err != nil {
return nil, err
}
return ScalarSubquery{Query: q}, nil
}

// Regular parenthesized expression like: (1 + 2)
// Parse the inner expression starting from the current position (after the "(")
e, err := p.parseExpr()
if err != nil {
return nil, err
Comment on lines 4661 to 4680

@gemini-code-assist gemini-code-assist Bot Oct 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This logic for handling parenthesized expressions and scalar subqueries appears to have a critical bug. The opening parenthesis ( is not consumed before p.parseQuery() or p.parseExpr() is called.

  1. For a scalar subquery like (SELECT ...), p.sniff("SELECT") will be true, but since the parser's position is still at (, the subsequent call to p.parseQuery() will likely fail as it expects to see a SELECT keyword.
  2. For a regular parenthesized expression like (1+2), p.sniff("SELECT") will be false, and execution will fall through to p.parseExpr(). Since the parser is still positioned at (, calling p.parseExpr() will eventually lead to a recursive call to p.parseLit(), which will see ( again, causing infinite recursion.

The fix is to consume the opening parenthesis ( before deciding which kind of expression to parse. The logic for the entire if tok.value == "(" block should be refactored to handle this correctly.

if tok.value == "(" {
		p.next() // Consume "(".

		// Check if this is a scalar subquery.
		tok := p.peek()
		if tok.kind == keyword && strings.EqualFold(tok.value, "SELECT") {
			q, err := p.parseQuery()
			if err != nil {
				return nil, err
			}
			if err := p.expect(")"); err != nil {
				return nil, err
			}
			return ScalarSubquery{Query: q}, nil
		}

		// It's a regular parenthesized expression.
		e, err := p.parseExpr()
		if err != nil {
			return nil, err
		}

@anicoll anicoll Oct 30, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this is correct, I have just pushed a commit which validates it with tests and added an extra comment.

Expand Down
195 changes: 195 additions & 0 deletions spanner/spansql/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,65 @@ func TestParseQuery(t *testing.T) {
},
},
},
// Scalar subqueries in SELECT list - ensures parseLit handles (SELECT ...) correctly
{
`SELECT (SELECT MAX(id) FROM t1), name FROM t2`,
Query{
Select: Select{
List: []Expr{
ScalarSubquery{
Query: Query{
Select: Select{
List: []Expr{Func{Name: "MAX", Args: []Expr{ID("id")}}},
From: []SelectFrom{SelectFromTable{Table: "t1"}},
},
},
},
ID("name"),
},
From: []SelectFrom{SelectFromTable{Table: "t2"}},
},
},
},
// Scalar subquery in WHERE clause
{
`SELECT * FROM users WHERE age > (SELECT AVG(age) FROM users)`,
Query{
Select: Select{
List: []Expr{Star},
From: []SelectFrom{SelectFromTable{Table: "users"}},
Where: ComparisonOp{
Op: Gt,
LHS: ID("age"),
RHS: ScalarSubquery{
Query: Query{
Select: Select{
List: []Expr{Func{Name: "AVG", Args: []Expr{ID("age")}}},
From: []SelectFrom{SelectFromTable{Table: "users"}},
},
},
},
},
},
},
},
// Parenthesized expression in SELECT list - ensures parseLit handles (expr) correctly
{
`SELECT (1 + 2) * 3, name FROM t`,
Query{
Select: Select{
List: []Expr{
ArithOp{
LHS: Paren{Expr: ArithOp{LHS: IntegerLiteral(1), Op: Add, RHS: IntegerLiteral(2)}},
Op: Mul,
RHS: IntegerLiteral(3),
},
ID("name"),
},
From: []SelectFrom{SelectFromTable{Table: "t"}},
},
},
},
}
for _, test := range tests {
got, err := ParseQuery(test.in)
Expand Down Expand Up @@ -634,6 +693,53 @@ func TestParseExpr(t *testing.T) {
// Reserved keywords.
{`TRUE AND FALSE`, LogicalOp{LHS: True, Op: And, RHS: False}},
{`NULL`, Null},

// Parenthesized expressions - test that parseLit correctly handles the opening paren.
// The opening "(" is consumed by p.next(), then parseExpr is called for the contents.
{`(1)`, Paren{Expr: IntegerLiteral(1)}},
{`(1 + 2)`, Paren{Expr: ArithOp{LHS: IntegerLiteral(1), Op: Add, RHS: IntegerLiteral(2)}}},
{`((1 + 2))`, Paren{Expr: Paren{Expr: ArithOp{LHS: IntegerLiteral(1), Op: Add, RHS: IntegerLiteral(2)}}}},
{`(1 + 2) * 3`, ArithOp{LHS: Paren{Expr: ArithOp{LHS: IntegerLiteral(1), Op: Add, RHS: IntegerLiteral(2)}}, Op: Mul, RHS: IntegerLiteral(3)}},
{`((1 + 2) * 3)`, Paren{Expr: ArithOp{LHS: Paren{Expr: ArithOp{LHS: IntegerLiteral(1), Op: Add, RHS: IntegerLiteral(2)}}, Op: Mul, RHS: IntegerLiteral(3)}}},
{`(A AND B) OR C`, LogicalOp{LHS: Paren{Expr: LogicalOp{LHS: ID("A"), Op: And, RHS: ID("B")}}, Op: Or, RHS: ID("C")}},
{`(TRUE)`, Paren{Expr: True}},
{`(NULL)`, Paren{Expr: Null}},

// Scalar subqueries - test that parseLit correctly distinguishes (SELECT ...) from (expr).
// When p.sniff("SELECT") is true after consuming "(", parseQuery is called instead of parseExpr.
{
`(SELECT 1)`,
ScalarSubquery{
Query: Query{
Select: Select{
List: []Expr{IntegerLiteral(1)},
},
},
},
},
{
`(SELECT MAX(x) FROM t)`,
ScalarSubquery{
Query: Query{
Select: Select{
List: []Expr{Func{Name: "MAX", Args: []Expr{ID("x")}}},
From: []SelectFrom{SelectFromTable{Table: "t"}},
},
},
},
},
{
`(SELECT COUNT(*) FROM users WHERE active = TRUE)`,
ScalarSubquery{
Query: Query{
Select: Select{
List: []Expr{Func{Name: "COUNT", Args: []Expr{Star}}},
From: []SelectFrom{SelectFromTable{Table: "users"}},
Where: ComparisonOp{LHS: ID("active"), Op: Eq, RHS: True},
},
},
},
},
}
for _, test := range tests {
p := newParser("test-file", test.in)
Expand Down Expand Up @@ -2415,6 +2521,95 @@ func TestParseDDL(t *testing.T) {
},
},
},
{
`CREATE OR REPLACE VIEW Transaction SQL SECURITY INVOKER AS SELECT
ID, Name, Amount, AccountID, PaymentID
FROM
Transactions as t
JOIN Accounts as acc ON t.AccountID = acc.ID
JOIN Payment as p ON t.PaymentID = p.ID
AND p.EventSequence = (
SELECT MAX(p2.EventSequence)
FROM Payment as p2
WHERE p2.ID = p.ID
)`,
&DDL{
Filename: "filename",
List: []DDLStmt{
&CreateView{
Name: "Transaction",
OrReplace: true,
SecurityType: Invoker,
Query: Query{
Select: Select{
List: []Expr{ID("ID"), ID("Name"), ID("Amount"), ID("AccountID"), ID("PaymentID")},
From: []SelectFrom{
SelectFromJoin{
Type: InnerJoin,
LHS: SelectFromJoin{
Type: InnerJoin,
LHS: SelectFromTable{
Table: "Transactions",
Alias: "t",
},
RHS: SelectFromTable{
Table: "Accounts",
Alias: "acc",
},
On: ComparisonOp{
LHS: PathExp{"t", "AccountID"},
Op: Eq,
RHS: PathExp{"acc", "ID"},
},
},
RHS: SelectFromTable{
Table: "Payment",
Alias: "p",
},
On: LogicalOp{
LHS: ComparisonOp{
LHS: PathExp{"t", "PaymentID"},
Op: Eq,
RHS: PathExp{"p", "ID"},
},
Op: And,
RHS: ComparisonOp{
LHS: PathExp{"p", "EventSequence"},
Op: Eq,
RHS: ScalarSubquery{
Query: Query{
Select: Select{
List: []Expr{
Func{
Name: "MAX",
Args: []Expr{PathExp{"p2", "EventSequence"}},
},
},
From: []SelectFrom{
SelectFromTable{
Table: "Payment",
Alias: "p2",
},
},
Where: ComparisonOp{
LHS: PathExp{"p2", "ID"},
Op: Eq,
RHS: PathExp{"p", "ID"},
},
},
},
},
},
},
},
},
},
},
Position: line(1),
},
},
},
},
}
for _, test := range tests {
got, err := ParseDDL("filename", test.in)
Expand Down
7 changes: 7 additions & 0 deletions spanner/spansql/sql.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,13 @@ func (eo ExistsOp) addSQL(sb *strings.Builder) {
sb.WriteString(")")
}

func (ss ScalarSubquery) SQL() string { return buildSQL(ss) }
func (ss ScalarSubquery) addSQL(sb *strings.Builder) {
sb.WriteString("(")
ss.Query.addSQL(sb)
sb.WriteString(")")
}

func (io InOp) SQL() string { return buildSQL(io) }
func (io InOp) addSQL(sb *strings.Builder) {
io.LHS.addSQL(sb)
Expand Down
6 changes: 6 additions & 0 deletions spanner/spansql/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,12 @@ type ExistsOp struct {
func (ExistsOp) isBoolExpr() {} // usually
func (ExistsOp) isExpr() {}

type ScalarSubquery struct {
Query Query
}

func (ScalarSubquery) isExpr() {}

type InOp struct {
LHS Expr
Neg bool
Expand Down
Loading