From 92cf14fb391ab0a161862cec5db3b510059938e1 Mon Sep 17 00:00:00 2001 From: Sharad Gaur Date: Thu, 20 Nov 2025 09:59:34 -0500 Subject: [PATCH 1/4] BugFix: Refreshable View with named parameters Signed-off-by: Sharad Gaur --- parser/ast.go | 34 ++++ parser/ast_visitor.go | 8 + parser/parser_column.go | 56 ++++++- parser/parser_table.go | 47 +++++- parser/parser_test.go | 18 +- .../ddl/create_materialized_view_with_gcs.sql | 3 + .../create_materialized_view_with_gcs.sql | 7 + ...materialized_view_with_gcs.sql.golden.json | 156 ++++++++++++++++++ 8 files changed, 318 insertions(+), 11 deletions(-) create mode 100644 parser/testdata/ddl/create_materialized_view_with_gcs.sql create mode 100644 parser/testdata/ddl/format/create_materialized_view_with_gcs.sql create mode 100644 parser/testdata/ddl/output/create_materialized_view_with_gcs.sql.golden.json diff --git a/parser/ast.go b/parser/ast.go index 8e85177..8cb13c9 100644 --- a/parser/ast.go +++ b/parser/ast.go @@ -4073,6 +4073,40 @@ func (m *MapLiteral) Accept(visitor ASTVisitor) error { return visitor.VisitMapLiteral(m) } +type NamedParameterExpr struct { + NamePos Pos + Name *Ident + Value Expr +} + +func (n *NamedParameterExpr) Pos() Pos { + return n.NamePos +} + +func (n *NamedParameterExpr) End() Pos { + return n.Value.End() +} + +func (n *NamedParameterExpr) String() string { + var builder strings.Builder + builder.WriteString(n.Name.String()) + builder.WriteByte('=') + builder.WriteString(n.Value.String()) + return builder.String() +} + +func (n *NamedParameterExpr) Accept(visitor ASTVisitor) error { + visitor.Enter(n) + defer visitor.Leave(n) + if err := n.Name.Accept(visitor); err != nil { + return err + } + if err := n.Value.Accept(visitor); err != nil { + return err + } + return visitor.VisitNamedParameterExpr(n) +} + type QueryParam struct { LBracePos Pos RBracePos Pos diff --git a/parser/ast_visitor.go b/parser/ast_visitor.go index b33aa49..ee8f066 100644 --- a/parser/ast_visitor.go +++ b/parser/ast_visitor.go @@ -85,6 +85,7 @@ type ASTVisitor interface { VisitSettingsExprList(expr *SettingsClause) error VisitParamExprList(expr *ParamExprList) error VisitMapLiteral(expr *MapLiteral) error + VisitNamedParameterExpr(expr *NamedParameterExpr) error VisitArrayParamList(expr *ArrayParamList) error VisitQueryParam(expr *QueryParam) error VisitObjectParams(expr *ObjectParams) error @@ -813,6 +814,13 @@ func (v *DefaultASTVisitor) VisitMapLiteral(expr *MapLiteral) error { return nil } +func (v *DefaultASTVisitor) VisitNamedParameterExpr(expr *NamedParameterExpr) error { + if v.Visit != nil { + return v.Visit(expr) + } + return nil +} + func (v *DefaultASTVisitor) VisitObjectParams(expr *ObjectParams) error { if v.Visit != nil { return v.Visit(expr) diff --git a/parser/parser_column.go b/parser/parser_column.go index c68d63f..828746f 100644 --- a/parser/parser_column.go +++ b/parser/parser_column.go @@ -518,7 +518,36 @@ func (p *Parser) parseColumnExprListWithTerm(term TokenKind, pos Pos) (*ColumnEx if term != "" && p.matchTokenKind(term) { break } - columnExpr, err := p.parseColumnsExpr(p.Pos()) + + // Check if this is a named parameter (identifier followed by =) + // We need to peek ahead to see if there's an identifier followed by = + var columnExpr Expr + var err error + + // Try to detect named parameter pattern: identifier = value + isNamedParam := false + firstToken, peekErr := p.lexer.peekToken() + if peekErr == nil && firstToken != nil && (firstToken.Kind == TokenKindIdent || firstToken.Kind == TokenKindKeyword) { + // Peek two tokens ahead to check for = + // Save state, consume first token, peek second, restore + savedState := p.lexer.saveState() + if err := p.lexer.consumeToken(); err == nil { + secondToken, peekErr2 := p.lexer.peekToken() + if peekErr2 == nil && secondToken != nil && secondToken.Kind == TokenKindSingleEQ { + isNamedParam = true + } + } + p.lexer.restoreState(savedState) + } + + if isNamedParam { + // Parse as named parameter + columnExpr, err = p.parseNamedParameter(p.Pos()) + } else { + // Parse as regular column expression + columnExpr, err = p.parseColumnsExpr(p.Pos()) + } + if err != nil { return nil, err } @@ -537,6 +566,31 @@ func (p *Parser) parseColumnExprListWithTerm(term TokenKind, pos Pos) (*ColumnEx return columnExprList, nil } +func (p *Parser) parseNamedParameter(pos Pos) (*NamedParameterExpr, error) { + // Parse the parameter name + name, err := p.parseIdent() + if err != nil { + return nil, err + } + + // Expect and consume the = token + if err := p.expectTokenKind(TokenKindSingleEQ); err != nil { + return nil, err + } + + // Parse the parameter value + value, err := p.parseExpr(p.Pos()) + if err != nil { + return nil, err + } + + return &NamedParameterExpr{ + NamePos: pos, + Name: name, + Value: value, + }, nil +} + func (p *Parser) parseSelectItems() ([]*SelectItem, error) { selectItems := make([]*SelectItem, 0) for !p.lexer.isEOF() || p.last() != nil { diff --git a/parser/parser_table.go b/parser/parser_table.go index e37eae6..a76ebba 100644 --- a/parser/parser_table.go +++ b/parser/parser_table.go @@ -649,7 +649,52 @@ func (p *Parser) parseTableArgList(pos Pos) (*TableArgListExpr, error) { args := make([]Expr, 0) for !p.lexer.isEOF() { - arg, err := p.parseTableArgExpr(p.Pos()) + // Check if this is a named parameter (identifier followed by =) + var arg Expr + var err error + + // Try to detect named parameter pattern: last token is identifier, next token is = + isNamedParam := false + lastKind := p.lastTokenKind() + if lastKind == TokenKindIdent || lastKind == TokenKindKeyword { + // Last token is an identifier, peek at the next token + nextToken, peekErr := p.lexer.peekToken() + + if peekErr == nil && nextToken != nil && nextToken.Kind == TokenKindSingleEQ { + isNamedParam = true + } + } + + if isNamedParam { + // Parse as named parameter - the identifier is already the last token + // We need to get it, consume the =, and parse the value + name := &Ident{ + NamePos: p.last().Pos, + NameEnd: p.last().End, + Name: p.last().String, + } + // Consume the = token + if err := p.lexer.consumeToken(); err != nil { + return nil, err + } + if err := p.expectTokenKind(TokenKindSingleEQ); err != nil { + return nil, err + } + // Parse the value + value, err := p.parseTableArgExpr(p.Pos()) + if err != nil { + return nil, err + } + arg = &NamedParameterExpr{ + NamePos: name.NamePos, + Name: name, + Value: value, + } + } else { + // Parse as regular table arg expression + arg, err = p.parseTableArgExpr(p.Pos()) + } + if err != nil { return nil, err } diff --git a/parser/parser_test.go b/parser/parser_test.go index 2175745..d0bb41a 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -132,15 +132,15 @@ func TestParser_InvalidSyntax(t *testing.T) { invalidSQLs := []string{ "SELECT * FROM", // WITH FILL error cases - "SELECT n FROM t ORDER BY n WITH", // WITH without FILL - "SELECT n FROM t ORDER BY n FILL", // FILL without WITH - "SELECT n FROM t ORDER BY n WITH FILL FROM", // FROM without value - "SELECT n FROM t ORDER BY n WITH FILL TO", // TO without value - "SELECT n FROM t ORDER BY n WITH FILL STEP", // STEP without value - "SELECT n FROM t ORDER BY n WITH FILL STALENESS", // STALENESS without value - "SELECT n FROM t ORDER BY n WITH FILL INTERPOLATE (x", // Missing closing paren - "SELECT n FROM t ORDER BY n WITH FILL INTERPOLATE x AS x + 1", // Missing parens around column list - "ALTER TABLE foo_mv MODIFY QUERY AS SELECT * FROM baz", // MODIFY QUERY followed by an invalid query + "SELECT n FROM t ORDER BY n WITH", // WITH without FILL + "SELECT n FROM t ORDER BY n FILL", // FILL without WITH + "SELECT n FROM t ORDER BY n WITH FILL FROM", // FROM without value + "SELECT n FROM t ORDER BY n WITH FILL TO", // TO without value + "SELECT n FROM t ORDER BY n WITH FILL STEP", // STEP without value + "SELECT n FROM t ORDER BY n WITH FILL STALENESS", // STALENESS without value + "SELECT n FROM t ORDER BY n WITH FILL INTERPOLATE (x", // Missing closing paren + "SELECT n FROM t ORDER BY n WITH FILL INTERPOLATE x AS x + 1", // Missing parens around column list + "ALTER TABLE foo_mv MODIFY QUERY AS SELECT * FROM baz", // MODIFY QUERY followed by an invalid query } for _, sql := range invalidSQLs { parser := NewParser(sql) diff --git a/parser/testdata/ddl/create_materialized_view_with_gcs.sql b/parser/testdata/ddl/create_materialized_view_with_gcs.sql new file mode 100644 index 0000000..0a664c0 --- /dev/null +++ b/parser/testdata/ddl/create_materialized_view_with_gcs.sql @@ -0,0 +1,3 @@ +CREATE MATERIALIZED VIEW database_name.view_name + REFRESH EVERY 5 MINUTE TO database_name.table_name AS + SELECT * FROM gcs(gcs_creds,url='https://storage.googleapis.com/some-bucket/some-path/'); \ No newline at end of file diff --git a/parser/testdata/ddl/format/create_materialized_view_with_gcs.sql b/parser/testdata/ddl/format/create_materialized_view_with_gcs.sql new file mode 100644 index 0000000..4974c12 --- /dev/null +++ b/parser/testdata/ddl/format/create_materialized_view_with_gcs.sql @@ -0,0 +1,7 @@ +-- Origin SQL: +CREATE MATERIALIZED VIEW database_name.view_name + REFRESH EVERY 5 MINUTE TO database_name.table_name AS + SELECT * FROM gcs(gcs_creds,url='https://storage.googleapis.com/some-bucket/some-path/'); + +-- Format SQL: +CREATE MATERIALIZED VIEW database_name.view_name REFRESH EVERY 5 MINUTE TO database_name.table_name AS SELECT * FROM gcs(gcs_creds, url='https://storage.googleapis.com/some-bucket/some-path/'); diff --git a/parser/testdata/ddl/output/create_materialized_view_with_gcs.sql.golden.json b/parser/testdata/ddl/output/create_materialized_view_with_gcs.sql.golden.json new file mode 100644 index 0000000..ecea3b6 --- /dev/null +++ b/parser/testdata/ddl/output/create_materialized_view_with_gcs.sql.golden.json @@ -0,0 +1,156 @@ +[ + { + "CreatePos": 0, + "StatementEnd": 206, + "Name": { + "Database": { + "Name": "database_name", + "QuoteType": 1, + "NamePos": 25, + "NameEnd": 38 + }, + "Table": { + "Name": "view_name", + "QuoteType": 1, + "NamePos": 39, + "NameEnd": 48 + } + }, + "IfNotExists": false, + "OnCluster": null, + "Refresh": { + "RefreshPos": 57, + "Frequency": "EVERY", + "Interval": { + "IntervalPos": 0, + "Expr": { + "NumPos": 71, + "NumEnd": 72, + "Literal": "5", + "Base": 10 + }, + "Unit": { + "Name": "MINUTE", + "QuoteType": 1, + "NamePos": 73, + "NameEnd": 79 + } + }, + "Offset": null + }, + "RandomizeFor": null, + "DependsOn": null, + "Settings": null, + "HasAppend": false, + "Engine": null, + "HasEmpty": false, + "Destination": { + "ToPos": 80, + "TableIdentifier": { + "Database": { + "Name": "database_name", + "QuoteType": 1, + "NamePos": 83, + "NameEnd": 96 + }, + "Table": { + "Name": "table_name", + "QuoteType": 1, + "NamePos": 97, + "NameEnd": 107 + } + }, + "TableSchema": null + }, + "SubQuery": { + "HasParen": false, + "Select": { + "SelectPos": 119, + "StatementEnd": 206, + "With": null, + "Top": null, + "HasDistinct": false, + "DistinctOn": null, + "SelectItems": [ + { + "Expr": { + "Name": "*", + "QuoteType": 0, + "NamePos": 126, + "NameEnd": 126 + }, + "Modifiers": [], + "Alias": null + } + ], + "From": { + "FromPos": 128, + "Expr": { + "Table": { + "TablePos": 133, + "TableEnd": 206, + "Alias": null, + "Expr": { + "Name": { + "Name": "gcs", + "QuoteType": 1, + "NamePos": 133, + "NameEnd": 136 + }, + "Args": { + "LeftParenPos": 136, + "RightParenPos": 206, + "Args": [ + { + "Name": "gcs_creds", + "QuoteType": 1, + "NamePos": 137, + "NameEnd": 146 + }, + { + "NamePos": 147, + "Name": { + "Name": "url", + "QuoteType": 0, + "NamePos": 147, + "NameEnd": 150 + }, + "Value": { + "LiteralPos": 152, + "LiteralEnd": 205, + "Literal": "https://storage.googleapis.com/some-bucket/some-path/" + } + } + ] + } + }, + "HasFinal": false + }, + "StatementEnd": 206, + "SampleRatio": null, + "HasFinal": false + } + }, + "ArrayJoin": null, + "Window": null, + "Prewhere": null, + "Where": null, + "GroupBy": null, + "WithTotal": false, + "Having": null, + "OrderBy": null, + "LimitBy": null, + "Limit": null, + "Settings": null, + "Format": null, + "UnionAll": null, + "UnionDistinct": null, + "Except": null + } + }, + "Populate": false, + "Comment": null, + "Definer": null, + "SQLSecurity": "" + } +] \ No newline at end of file From 3ba1616951e5a771aec5b49e9e81fa4069009a98 Mon Sep 17 00:00:00 2001 From: Sharad Gaur Date: Thu, 20 Nov 2025 10:12:46 -0500 Subject: [PATCH 2/4] Fixing Signed-off-by: Sharad Gaur --- parser/parser_column.go | 31 +------------------ parser/parser_table.go | 8 ++--- .../1_stateful/00177_select_from_gcs.sql | 1 + 3 files changed, 6 insertions(+), 34 deletions(-) create mode 100644 parser/testdata/query/compatible/1_stateful/00177_select_from_gcs.sql diff --git a/parser/parser_column.go b/parser/parser_column.go index 828746f..e320e3e 100644 --- a/parser/parser_column.go +++ b/parser/parser_column.go @@ -518,36 +518,7 @@ func (p *Parser) parseColumnExprListWithTerm(term TokenKind, pos Pos) (*ColumnEx if term != "" && p.matchTokenKind(term) { break } - - // Check if this is a named parameter (identifier followed by =) - // We need to peek ahead to see if there's an identifier followed by = - var columnExpr Expr - var err error - - // Try to detect named parameter pattern: identifier = value - isNamedParam := false - firstToken, peekErr := p.lexer.peekToken() - if peekErr == nil && firstToken != nil && (firstToken.Kind == TokenKindIdent || firstToken.Kind == TokenKindKeyword) { - // Peek two tokens ahead to check for = - // Save state, consume first token, peek second, restore - savedState := p.lexer.saveState() - if err := p.lexer.consumeToken(); err == nil { - secondToken, peekErr2 := p.lexer.peekToken() - if peekErr2 == nil && secondToken != nil && secondToken.Kind == TokenKindSingleEQ { - isNamedParam = true - } - } - p.lexer.restoreState(savedState) - } - - if isNamedParam { - // Parse as named parameter - columnExpr, err = p.parseNamedParameter(p.Pos()) - } else { - // Parse as regular column expression - columnExpr, err = p.parseColumnsExpr(p.Pos()) - } - + columnExpr, err := p.parseColumnsExpr(p.Pos()) if err != nil { return nil, err } diff --git a/parser/parser_table.go b/parser/parser_table.go index a76ebba..55d7c47 100644 --- a/parser/parser_table.go +++ b/parser/parser_table.go @@ -652,19 +652,19 @@ func (p *Parser) parseTableArgList(pos Pos) (*TableArgListExpr, error) { // Check if this is a named parameter (identifier followed by =) var arg Expr var err error - + // Try to detect named parameter pattern: last token is identifier, next token is = isNamedParam := false lastKind := p.lastTokenKind() if lastKind == TokenKindIdent || lastKind == TokenKindKeyword { // Last token is an identifier, peek at the next token nextToken, peekErr := p.lexer.peekToken() - + if peekErr == nil && nextToken != nil && nextToken.Kind == TokenKindSingleEQ { isNamedParam = true } } - + if isNamedParam { // Parse as named parameter - the identifier is already the last token // We need to get it, consume the =, and parse the value @@ -694,7 +694,7 @@ func (p *Parser) parseTableArgList(pos Pos) (*TableArgListExpr, error) { // Parse as regular table arg expression arg, err = p.parseTableArgExpr(p.Pos()) } - + if err != nil { return nil, err } diff --git a/parser/testdata/query/compatible/1_stateful/00177_select_from_gcs.sql b/parser/testdata/query/compatible/1_stateful/00177_select_from_gcs.sql new file mode 100644 index 0000000..de4ca4a --- /dev/null +++ b/parser/testdata/query/compatible/1_stateful/00177_select_from_gcs.sql @@ -0,0 +1 @@ +SELECT * FROM gcs(gcs_creds,url='https://storage.googleapis.com/some-bucket/some-path/'); \ No newline at end of file From fe17149611aa10955cfc4c154dd1babfc6649218 Mon Sep 17 00:00:00 2001 From: Sharad Gaur Date: Thu, 20 Nov 2025 10:16:42 -0500 Subject: [PATCH 3/4] Removing unwanted code Signed-off-by: Sharad Gaur --- parser/parser_column.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/parser/parser_column.go b/parser/parser_column.go index e320e3e..c68d63f 100644 --- a/parser/parser_column.go +++ b/parser/parser_column.go @@ -537,31 +537,6 @@ func (p *Parser) parseColumnExprListWithTerm(term TokenKind, pos Pos) (*ColumnEx return columnExprList, nil } -func (p *Parser) parseNamedParameter(pos Pos) (*NamedParameterExpr, error) { - // Parse the parameter name - name, err := p.parseIdent() - if err != nil { - return nil, err - } - - // Expect and consume the = token - if err := p.expectTokenKind(TokenKindSingleEQ); err != nil { - return nil, err - } - - // Parse the parameter value - value, err := p.parseExpr(p.Pos()) - if err != nil { - return nil, err - } - - return &NamedParameterExpr{ - NamePos: pos, - Name: name, - Value: value, - }, nil -} - func (p *Parser) parseSelectItems() ([]*SelectItem, error) { selectItems := make([]*SelectItem, 0) for !p.lexer.isEOF() || p.last() != nil { From f8c2a1fb9d3343f849063135be606f3a61ffc7f9 Mon Sep 17 00:00:00 2001 From: Sharad Gaur Date: Thu, 20 Nov 2025 21:07:09 -0500 Subject: [PATCH 4/4] Adding to walk.go Signed-off-by: Sharad Gaur --- parser/walk.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/parser/walk.go b/parser/walk.go index 9bac61b..a69fde3 100644 --- a/parser/walk.go +++ b/parser/walk.go @@ -1319,6 +1319,13 @@ func Walk(node Expr, fn WalkFunc) bool { return false } } + case *NamedParameterExpr: + if !Walk(n.Name, fn) { + return false + } + if !Walk(n.Value, fn) { + return false + } case *ObjectParams: if !Walk(n.Object, fn) { return false