Skip to content

Commit 64d3d75

Browse files
kyleconroykyleconroy
authored
Fix 5 parser tests with PRIMARY KEY, CREATE VIEW validation, and negated literals (#47)
1 parent e2d74d1 commit 64d3d75

File tree

18 files changed

+111
-21
lines changed

18 files changed

+111
-21
lines changed

‎ast/ast.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ type CreateQuery struct {
258258
TTL *TTLClause `json:"ttl,omitempty"`
259259
Settings []*SettingExpr `json:"settings,omitempty"`
260260
AsSelect Statement `json:"as_select,omitempty"`
261+
AsTableFunction Expression `json:"as_table_function,omitempty"` // AS table_function(...) in CREATE TABLE
261262
Comment string `json:"comment,omitempty"`
262263
OnCluster string `json:"on_cluster,omitempty"`
263264
CreateDatabase bool `json:"create_database,omitempty"`

‎internal/explain/explain.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import (
88
"github.com/sqlc-dev/doubleclick/ast"
99
)
1010

11+
// inSubqueryContext is a package-level flag to track when we're inside a Subquery
12+
// This affects how negated literals with aliases are formatted
13+
var inSubqueryContext bool
14+
1115
// Explain returns the EXPLAIN AST output for a statement, matching ClickHouse's format.
1216
func Explain(stmt ast.Statement) string {
1317
var sb strings.Builder

‎internal/explain/expressions.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,12 @@ func explainSubquery(sb *strings.Builder, n *ast.Subquery, indent string, depth
314314
} else {
315315
fmt.Fprintf(sb, "%sSubquery (children %d)\n", indent, children)
316316
}
317+
// Set context flag before recursing into subquery content
318+
// This affects how negated literals with aliases are formatted
319+
prevContext := inSubqueryContext
320+
inSubqueryContext = true
317321
Node(sb, n.Query, depth+1)
322+
inSubqueryContext = prevContext
318323
}
319324

320325
func explainAliasedExpr(sb *strings.Builder, n *ast.AliasedExpr, depth int) {
@@ -398,6 +403,28 @@ func explainAliasedExpr(sb *strings.Builder, n *ast.AliasedExpr, depth int) {
398403
Node(sb, e.Right, depth+2)
399404
}
400405
case *ast.UnaryExpr:
406+
// When inside a Subquery context, negated numeric literals should be output as Literal Int64_-N
407+
// Otherwise, output as Function negate
408+
if inSubqueryContext && e.Op == "-" {
409+
if lit, ok := e.Operand.(*ast.Literal); ok {
410+
switch lit.Type {
411+
case ast.LiteralInteger:
412+
switch val := lit.Value.(type) {
413+
case int64:
414+
fmt.Fprintf(sb, "%sLiteral Int64_%d (alias %s)\n", indent, -val, n.Alias)
415+
return
416+
case uint64:
417+
fmt.Fprintf(sb, "%sLiteral Int64_-%d (alias %s)\n", indent, val, n.Alias)
418+
return
419+
}
420+
case ast.LiteralFloat:
421+
val := lit.Value.(float64)
422+
s := FormatFloat(-val)
423+
fmt.Fprintf(sb, "%sLiteral Float64_%s (alias %s)\n", indent, s, n.Alias)
424+
return
425+
}
426+
}
427+
}
401428
// Unary expressions become functions with alias
402429
fnName := UnaryOperatorToFunction(e.Op)
403430
fmt.Fprintf(sb, "%sFunction %s (alias %s) (children %d)\n", indent, fnName, n.Alias, 1)

‎internal/explain/functions.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,13 @@ func explainFunctionCallWithAlias(sb *strings.Builder, n *ast.FunctionCall, alia
4242
fmt.Fprintln(sb)
4343
for _, arg := range n.Arguments {
4444
// For view() table function, unwrap Subquery wrapper
45+
// Also reset the subquery context since view() SELECT is not in a Subquery node
4546
if strings.ToLower(n.Name) == "view" {
4647
if sq, ok := arg.(*ast.Subquery); ok {
48+
prevContext := inSubqueryContext
49+
inSubqueryContext = false
4750
Node(sb, sq.Query, depth+2)
51+
inSubqueryContext = prevContext
4852
continue
4953
}
5054
}

‎internal/explain/statements.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ func explainCreateQuery(sb *strings.Builder, n *ast.CreateQuery, indent string,
9797
if n.AsSelect != nil {
9898
children++
9999
}
100+
if n.AsTableFunction != nil {
101+
children++
102+
}
100103
// ClickHouse adds an extra space before (children N) for CREATE DATABASE
101104
if n.CreateDatabase {
102105
fmt.Fprintf(sb, "%sCreateQuery %s (children %d)\n", indent, name, children)
@@ -112,6 +115,16 @@ func explainCreateQuery(sb *strings.Builder, n *ast.CreateQuery, indent string,
112115
if len(n.Indexes) > 0 {
113116
childrenCount++
114117
}
118+
// Check for PRIMARY KEY constraints in column declarations
119+
var primaryKeyColumns []string
120+
for _, col := range n.Columns {
121+
if col.PrimaryKey {
122+
primaryKeyColumns = append(primaryKeyColumns, col.Name)
123+
}
124+
}
125+
if len(primaryKeyColumns) > 0 {
126+
childrenCount++ // Add for Function tuple containing PRIMARY KEY columns
127+
}
115128
fmt.Fprintf(sb, "%s Columns definition (children %d)\n", indent, childrenCount)
116129
if len(n.Columns) > 0 {
117130
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.Columns))
@@ -125,6 +138,14 @@ func explainCreateQuery(sb *strings.Builder, n *ast.CreateQuery, indent string,
125138
Index(sb, idx, depth+3)
126139
}
127140
}
141+
// Output PRIMARY KEY columns as Function tuple
142+
if len(primaryKeyColumns) > 0 {
143+
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
144+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(primaryKeyColumns))
145+
for _, colName := range primaryKeyColumns {
146+
fmt.Fprintf(sb, "%s Identifier %s\n", indent, colName)
147+
}
148+
}
128149
}
129150
if n.Engine != nil || len(n.OrderBy) > 0 || len(n.PrimaryKey) > 0 || n.PartitionBy != nil || len(n.Settings) > 0 {
130151
storageChildren := 0
@@ -228,6 +249,10 @@ func explainCreateQuery(sb *strings.Builder, n *ast.CreateQuery, indent string,
228249
// AS SELECT is output directly without Subquery wrapper
229250
Node(sb, n.AsSelect, depth+1)
230251
}
252+
if n.AsTableFunction != nil {
253+
// AS table_function(...) is output directly
254+
Node(sb, n.AsTableFunction, depth+1)
255+
}
231256
}
232257

233258
func explainDropQuery(sb *strings.Builder, n *ast.DropQuery, indent string, depth int) {
@@ -337,6 +362,27 @@ func explainShowQuery(sb *strings.Builder, n *ast.ShowQuery, indent string) {
337362
if showType == "Settings" || showType == "Databases" {
338363
showType = "Tables"
339364
}
365+
366+
// SHOW CREATE TABLE has special output format with database and table identifiers
367+
if n.ShowType == ast.ShowCreate && (n.Database != "" || n.From != "") {
368+
// Format: ShowCreateTableQuery database table (children 2)
369+
name := n.From
370+
if n.Database != "" && n.From != "" {
371+
fmt.Fprintf(sb, "%sShowCreateTableQuery %s %s (children 2)\n", indent, n.Database, n.From)
372+
fmt.Fprintf(sb, "%s Identifier %s\n", indent, n.Database)
373+
fmt.Fprintf(sb, "%s Identifier %s\n", indent, n.From)
374+
} else if n.From != "" {
375+
fmt.Fprintf(sb, "%sShowCreateTableQuery %s (children 1)\n", indent, name)
376+
fmt.Fprintf(sb, "%s Identifier %s\n", indent, name)
377+
} else if n.Database != "" {
378+
fmt.Fprintf(sb, "%sShowCreateTableQuery %s (children 1)\n", indent, n.Database)
379+
fmt.Fprintf(sb, "%s Identifier %s\n", indent, n.Database)
380+
} else {
381+
fmt.Fprintf(sb, "%sShow%s\n", indent, showType)
382+
}
383+
return
384+
}
385+
340386
fmt.Fprintf(sb, "%sShow%s\n", indent, showType)
341387
}
342388

‎internal/explain/tables.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,11 @@ func explainTableExpression(sb *strings.Builder, n *ast.TableExpression, indent
4141
explainViewExplain(sb, explainQ, n.Alias, indent+" ", depth+1)
4242
} else if n.Alias != "" {
4343
fmt.Fprintf(sb, "%s Subquery (alias %s) (children %d)\n", indent, n.Alias, 1)
44+
// Set context flag for subquery - affects how negated literals with aliases are formatted
45+
prevContext := inSubqueryContext
46+
inSubqueryContext = true
4447
Node(sb, subq.Query, depth+2)
48+
inSubqueryContext = prevContext
4549
} else {
4650
Node(sb, n.Table, depth+1)
4751
}

‎parser/expression.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ func (p *Parser) parseFunctionCall(name string, pos token.Position) *ast.Functio
506506
}
507507

508508
// Handle view() and similar functions that take a subquery as argument
509-
// view(SELECT ...) should parse SELECT as a subquery, not expression
509+
// view(SELECT ...) should parse SELECT as a subquery
510510
if strings.ToLower(name) == "view" && (p.currentIs(token.SELECT) || p.currentIs(token.WITH)) {
511511
subquery := p.parseSelectWithUnion()
512512
fn.Arguments = []ast.Expression{&ast.Subquery{Position: pos, Query: subquery}}

‎parser/parser.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,17 +1500,16 @@ done_table_options:
15001500
p.nextToken()
15011501
p.parseIdentifierName()
15021502
} else if p.currentIs(token.LPAREN) {
1503-
// AS function(...) - skip the function call
1504-
depth := 1
1505-
p.nextToken()
1506-
for depth > 0 && !p.currentIs(token.EOF) {
1507-
if p.currentIs(token.LPAREN) {
1508-
depth++
1509-
} else if p.currentIs(token.RPAREN) {
1510-
depth--
1511-
}
1503+
// AS function(...) - parse as a function call
1504+
fn := &ast.FunctionCall{Name: name}
1505+
p.nextToken() // skip (
1506+
if !p.currentIs(token.RPAREN) {
1507+
fn.Arguments = p.parseExpressionList()
1508+
}
1509+
if p.currentIs(token.RPAREN) {
15121510
p.nextToken()
15131511
}
1512+
create.AsTableFunction = fn
15141513
}
15151514
_ = name // Use name for future AS table support
15161515
}
@@ -1595,8 +1594,13 @@ func (p *Parser) parseCreateView(create *ast.CreateQuery) {
15951594
}
15961595
}
15971596

1598-
// Handle TO (target table for materialized views)
1597+
// Handle TO (target table for materialized views only)
1598+
// TO clause is not valid for regular views - only for MATERIALIZED VIEW
15991599
if p.currentIs(token.TO) {
1600+
if !create.Materialized {
1601+
p.errors = append(p.errors, fmt.Errorf("TO clause is only valid for MATERIALIZED VIEW, not VIEW"))
1602+
return
1603+
}
16001604
p.nextToken()
16011605
create.To = p.parseIdentifierName()
16021606
}

‎parser/testdata/00098_j_union_all/metadata.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}

‎parser/testdata/01668_avg_weighted_ubsan/metadata.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"todo": true}
1+
{}

0 commit comments

Comments
 (0)