Skip to content

Commit fc312ee

Browse files
Ajit Pratap SinghAjit Pratap Singh
authored andcommitted
Merge remote-tracking branch 'origin/main' into feat/snowflake-time-travel-483
# Conflicts: # pkg/sql/parser/select_subquery.go
2 parents d68d731 + d72b4d8 commit fc312ee

File tree

5 files changed

+154
-4
lines changed

5 files changed

+154
-4
lines changed
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2026 GoSQLX Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
5+
package parser_test
6+
7+
import (
8+
"testing"
9+
10+
"github.com/ajitpratap0/GoSQLX/pkg/gosqlx"
11+
"github.com/ajitpratap0/GoSQLX/pkg/sql/keywords"
12+
)
13+
14+
// TestClickHouseOrderByWithFill verifies ClickHouse's `ORDER BY expr WITH
15+
// FILL [FROM..] [TO..] [STEP..]` modifier parses. Previously the WITH token
16+
// after ORDER BY items was mis-routed to the WITH-CTE parser. Regression
17+
// for #482.
18+
func TestClickHouseOrderByWithFill(t *testing.T) {
19+
queries := map[string]string{
20+
"bare": `SELECT id FROM t ORDER BY id WITH FILL`,
21+
"step_only": `SELECT id FROM t ORDER BY id WITH FILL STEP 1`,
22+
"from_to_step": `SELECT day, count() FROM events GROUP BY day ORDER BY day WITH FILL FROM '2024-01-01' TO '2024-12-31' STEP INTERVAL 1 DAY`,
23+
"multiple_cols": `SELECT a, b FROM t ORDER BY a WITH FILL STEP 1, b`,
24+
"with_desc": `SELECT id FROM t ORDER BY id DESC WITH FILL STEP 1`,
25+
}
26+
for name, q := range queries {
27+
q := q
28+
t.Run(name, func(t *testing.T) {
29+
if _, err := gosqlx.ParseWithDialect(q, keywords.DialectClickHouse); err != nil {
30+
t.Fatalf("parse failed: %v", err)
31+
}
32+
})
33+
}
34+
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Copyright 2026 GoSQLX Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
5+
package parser_test
6+
7+
import (
8+
"testing"
9+
10+
"github.com/ajitpratap0/GoSQLX/pkg/gosqlx"
11+
"github.com/ajitpratap0/GoSQLX/pkg/sql/ast"
12+
"github.com/ajitpratap0/GoSQLX/pkg/sql/keywords"
13+
)
14+
15+
// TestMinusAsExceptSynonym verifies MINUS is accepted as a set operator
16+
// (synonym for EXCEPT) in Snowflake and Oracle dialects, and that it is
17+
// normalized to "EXCEPT" on the AST. Regression for #483.
18+
func TestMinusAsExceptSynonym(t *testing.T) {
19+
dialects := []keywords.SQLDialect{
20+
keywords.DialectSnowflake,
21+
keywords.DialectOracle,
22+
}
23+
q := `SELECT id FROM a MINUS SELECT id FROM b`
24+
for _, d := range dialects {
25+
d := d
26+
t.Run(string(d), func(t *testing.T) {
27+
tree, err := gosqlx.ParseWithDialect(q, d)
28+
if err != nil {
29+
t.Fatalf("parse failed: %v", err)
30+
}
31+
if len(tree.Statements) != 1 {
32+
t.Fatalf("want 1 statement, got %d", len(tree.Statements))
33+
}
34+
so, ok := tree.Statements[0].(*ast.SetOperation)
35+
if !ok {
36+
t.Fatalf("want *ast.SetOperation, got %T", tree.Statements[0])
37+
}
38+
if so.Operator != "EXCEPT" {
39+
t.Fatalf("Operator: want %q, got %q", "EXCEPT", so.Operator)
40+
}
41+
})
42+
}
43+
}
44+
45+
// TestMinusNotSetOpInOtherDialects verifies that MINUS is still treated as
46+
// a table alias (not a set operator) in dialects that do not support it.
47+
// This protects against accidental hijacking in e.g. MySQL/PostgreSQL.
48+
func TestMinusNotSetOpInOtherDialects(t *testing.T) {
49+
q := `SELECT id FROM a MINUS SELECT id FROM b`
50+
// In dialects without MINUS-as-EXCEPT, the MINUS identifier is consumed
51+
// as a table alias ("a AS MINUS") and "SELECT ..." starts a new statement.
52+
// We expect either success with 2 statements, or at minimum no panic.
53+
tree, err := gosqlx.ParseWithDialect(q, keywords.DialectPostgreSQL)
54+
if err != nil {
55+
return // error is acceptable; we only verify no hijacking
56+
}
57+
if len(tree.Statements) == 1 {
58+
if _, isSetOp := tree.Statements[0].(*ast.SetOperation); isSetOp {
59+
t.Fatal("PostgreSQL should not parse MINUS as a set operator")
60+
}
61+
}
62+
}

pkg/sql/parser/select_clauses.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,17 @@ func (p *Parser) parseOrderByClause() ([]ast.OrderByExpression, error) {
548548
}
549549
entry.NullsFirst = nullsFirst
550550

551+
// ClickHouse WITH FILL per-entry tail:
552+
// ORDER BY expr [ASC|DESC] WITH FILL [FROM x] [TO y] [STEP z]
553+
// Consume permissively; the clause is not modeled on the AST yet.
554+
if p.dialect == string(keywords.DialectClickHouse) &&
555+
p.isType(models.TokenTypeWith) &&
556+
strings.EqualFold(p.peekToken().Token.Value, "FILL") {
557+
p.advance() // WITH
558+
p.advance() // FILL
559+
p.skipClickHouseWithFillTail()
560+
}
561+
551562
orderByExprs = append(orderByExprs, entry)
552563

553564
if !p.isType(models.TokenTypeComma) {
@@ -558,6 +569,26 @@ func (p *Parser) parseOrderByClause() ([]ast.OrderByExpression, error) {
558569
return orderByExprs, nil
559570
}
560571

572+
// skipClickHouseWithFillTail consumes the optional FROM / TO / STEP arguments
573+
// of a ClickHouse "ORDER BY expr WITH FILL" modifier. Each argument is a
574+
// single expression (possibly an INTERVAL). The tail ends at the next comma
575+
// (more ORDER BY items), next clause keyword, ';', or EOF.
576+
func (p *Parser) skipClickHouseWithFillTail() {
577+
for {
578+
val := strings.ToUpper(p.currentToken.Token.Value)
579+
if val != "FROM" && val != "TO" && val != "STEP" {
580+
return
581+
}
582+
p.advance() // FROM / TO / STEP
583+
// Consume one expression; ignore parse errors so unusual forms
584+
// (INTERVAL '1 day', expressions with function calls, etc.) don't
585+
// surface as parser errors for this permissive skip.
586+
if _, err := p.parseExpression(); err != nil {
587+
return
588+
}
589+
}
590+
}
591+
561592
// parseLimitOffsetClause parses optional LIMIT and/or OFFSET clauses.
562593
// Supports standard "LIMIT n OFFSET m", MySQL "LIMIT offset, count", and
563594
// SQL-99 "OFFSET n ROWS" (ROW/ROWS consumed but value stored).

pkg/sql/parser/select_set_ops.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@ package parser
1919

2020
import (
2121
"fmt"
22+
"strings"
2223

2324
goerrors "github.com/ajitpratap0/GoSQLX/pkg/errors"
2425
"github.com/ajitpratap0/GoSQLX/pkg/models"
2526
"github.com/ajitpratap0/GoSQLX/pkg/sql/ast"
27+
"github.com/ajitpratap0/GoSQLX/pkg/sql/keywords"
2628
)
2729

2830
// parseSelectWithSetOperations parses SELECT statements that may have set operations.
@@ -41,10 +43,16 @@ func (p *Parser) parseSelectWithSetOperations() (ast.Statement, error) {
4143
return nil, err
4244
}
4345

44-
// Check for set operations (UNION, EXCEPT, INTERSECT)
45-
for p.isAnyType(models.TokenTypeUnion, models.TokenTypeExcept, models.TokenTypeIntersect) {
46+
// Check for set operations (UNION, EXCEPT, INTERSECT). MINUS is a
47+
// Snowflake / Oracle synonym for EXCEPT; it tokenizes as a plain
48+
// identifier, so match it by value.
49+
for p.isAnyType(models.TokenTypeUnion, models.TokenTypeExcept, models.TokenTypeIntersect) ||
50+
p.isMinusSetOp() {
4651
// Parse the set operation type
4752
operationLiteral := p.currentToken.Token.Value
53+
if p.isMinusSetOp() {
54+
operationLiteral = "EXCEPT" // normalize to canonical name
55+
}
4856
p.advance()
4957

5058
// Check for ALL keyword
@@ -83,3 +91,18 @@ func (p *Parser) parseSelectWithSetOperations() (ast.Statement, error) {
8391

8492
return leftStmt, nil
8593
}
94+
95+
// isMinusSetOp returns true if the current token is the Snowflake / Oracle
96+
// MINUS keyword used as a set operator (synonym for EXCEPT). MINUS tokenizes
97+
// as an identifier; gate by dialect so that MINUS as a legitimate column
98+
// alias in other dialects is not hijacked.
99+
func (p *Parser) isMinusSetOp() bool {
100+
if p.dialect != string(keywords.DialectSnowflake) &&
101+
p.dialect != string(keywords.DialectOracle) {
102+
return false
103+
}
104+
if !p.isIdentifier() {
105+
return false
106+
}
107+
return strings.EqualFold(p.currentToken.Token.Value, "MINUS")
108+
}

pkg/sql/parser/select_subquery.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func (p *Parser) parseFromTableReference() (ast.TableReference, error) {
113113
// Similarly, START followed by WITH is a hierarchical query seed, not an alias.
114114
// Don't consume PIVOT/UNPIVOT as a table alias — they are contextual
115115
// keywords in SQL Server/Oracle and must reach the pivot-clause parser below.
116-
if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isSnowflakeTimeTravelStart() {
116+
if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() {
117117
if p.isType(models.TokenTypeAs) {
118118
p.advance() // Consume AS
119119
if !p.isIdentifier() {
@@ -227,7 +227,7 @@ func (p *Parser) parseJoinedTableRef(joinType string) (ast.TableReference, error
227227
// Similarly, START followed by WITH is a hierarchical query seed, not an alias.
228228
// Don't consume PIVOT/UNPIVOT as a table alias — they are contextual
229229
// keywords in SQL Server/Oracle and must reach the pivot-clause parser below.
230-
if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isSnowflakeTimeTravelStart() {
230+
if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() {
231231
if p.isType(models.TokenTypeAs) {
232232
p.advance()
233233
if !p.isIdentifier() {

0 commit comments

Comments
 (0)