Skip to content

Commit 8a5092d

Browse files
authored
Fix 67 parser issues: DROP TABLE multiple tables, negative literals, empty tuple ORDER BY (#14)
1 parent 4c69cda commit 8a5092d

8 files changed

Lines changed: 238 additions & 84 deletions

File tree

TODO.md

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
## Current State
44

5-
- **Tests passing:** 5,933 (86.9%)
6-
- **Tests skipped:** 891 (13.1%)
5+
- **Tests passing:** 6,006 (88.0%)
6+
- **Tests skipped:** 819 (12.0%)
77

88
## Recently Fixed (explain layer)
99

@@ -18,27 +18,15 @@
1818
- ✅ Aliased expression handling for binary/unary/function/identifier
1919
- ✅ PARTITION BY support in CREATE TABLE
2020
- ✅ Server error message stripping from expected output
21+
- ✅ DROP TABLE with multiple tables (e.g., `DROP TABLE t1, t2, t3`)
22+
- ✅ Negative integer/float literals (e.g., `-1``Literal Int64_-1`)
23+
- ✅ Empty tuple in ORDER BY (e.g., `ORDER BY ()``Function tuple` with empty `ExpressionList`)
24+
- ✅ String escape handling (lexer now unescapes `\'`, `\\`, `\n`, `\t`, `\0`, etc.)
2125

2226
## Parser Issues (High Priority)
2327

2428
These require changes to `parser/parser.go`:
2529

26-
### DROP TABLE with Multiple Tables
27-
Parser only captures first table when multiple are specified:
28-
```sql
29-
DROP TABLE IF EXISTS t1, t2, t3;
30-
-- Expected: ExpressionList with 3 TableIdentifiers
31-
-- Got: Single Identifier for t1
32-
```
33-
34-
### Negative Integer Literals
35-
Negative numbers are parsed as `Function negate` instead of negative literals:
36-
```sql
37-
SELECT -1, -10000;
38-
-- Expected: Literal Int64_-1
39-
-- Got: Function negate (children 1) with Literal UInt64_1
40-
```
41-
4230
### CREATE TABLE with INDEX Clause
4331
INDEX definitions in CREATE TABLE are not captured:
4432
```sql
@@ -59,22 +47,6 @@ CREATE TABLE t (c Int TTL expr()) ENGINE=MergeTree;
5947
-- Expected: ColumnDeclaration with 2 children (type + TTL function)
6048
```
6149

62-
### Empty Tuple in ORDER BY
63-
`ORDER BY ()` should capture empty tuple expression:
64-
```sql
65-
CREATE TABLE t (...) ENGINE=MergeTree ORDER BY ();
66-
-- Expected: Function tuple (children 1) with empty ExpressionList
67-
-- Got: Storage definition with no ORDER BY
68-
```
69-
70-
### String Escape Handling
71-
Parser stores escaped characters literally instead of unescaping:
72-
```sql
73-
SELECT 'x\'e2\'';
74-
-- Parser stores: x\'e2\' (with backslashes)
75-
-- Should store: x'e2' (unescaped)
76-
```
77-
7850
## Parser Issues (Medium Priority)
7951

8052
### CREATE DICTIONARY
@@ -163,11 +135,11 @@ SELECT 2.2250738585072014e-308;
163135
```
164136

165137
### Array Literals with Negative Numbers
166-
Arrays with negative integers expand to Function instead of Literal:
138+
Arrays with negative integers may still expand to Function instead of Literal in some cases:
167139
```sql
168140
SELECT [-10000, 5750];
169-
-- Expected: Literal Array_[Int64_-10000, UInt64_5750]
170-
-- Got: Function array with Function negate for -10000
141+
-- Some cases now work correctly with Literal Int64_-10000
142+
-- Complex nested arrays may still require additional work
171143
```
172144

173145
### WithElement for CTE Subqueries

ast/ast.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -328,16 +328,17 @@ func (t *TTLClause) End() token.Position { return t.Position }
328328

329329
// DropQuery represents a DROP statement.
330330
type DropQuery struct {
331-
Position token.Position `json:"-"`
332-
IfExists bool `json:"if_exists,omitempty"`
333-
Database string `json:"database,omitempty"`
334-
Table string `json:"table,omitempty"`
335-
View string `json:"view,omitempty"`
336-
User string `json:"user,omitempty"`
337-
Temporary bool `json:"temporary,omitempty"`
338-
OnCluster string `json:"on_cluster,omitempty"`
339-
DropDatabase bool `json:"drop_database,omitempty"`
340-
Sync bool `json:"sync,omitempty"`
331+
Position token.Position `json:"-"`
332+
IfExists bool `json:"if_exists,omitempty"`
333+
Database string `json:"database,omitempty"`
334+
Table string `json:"table,omitempty"`
335+
Tables []*TableIdentifier `json:"tables,omitempty"` // For DROP TABLE t1, t2, t3
336+
View string `json:"view,omitempty"`
337+
User string `json:"user,omitempty"`
338+
Temporary bool `json:"temporary,omitempty"`
339+
OnCluster string `json:"on_cluster,omitempty"`
340+
DropDatabase bool `json:"drop_database,omitempty"`
341+
Sync bool `json:"sync,omitempty"`
341342
}
342343

343344
func (d *DropQuery) Pos() token.Position { return d.Position }

internal/explain/explain.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func Node(sb *strings.Builder, node interface{}, depth int) {
104104
case *ast.CreateQuery:
105105
explainCreateQuery(sb, n, indent, depth)
106106
case *ast.DropQuery:
107-
explainDropQuery(sb, n, indent)
107+
explainDropQuery(sb, n, indent, depth)
108108
case *ast.SetQuery:
109109
explainSetQuery(sb, indent)
110110
case *ast.SystemQuery:

internal/explain/expressions.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,29 @@ func collectConcatOperands(n *ast.BinaryExpr) []ast.Expression {
132132
}
133133

134134
func explainUnaryExpr(sb *strings.Builder, n *ast.UnaryExpr, indent string, depth int) {
135+
// Handle negate of literal numbers - output as negative literal instead of function
136+
if n.Op == "-" {
137+
if lit, ok := n.Operand.(*ast.Literal); ok {
138+
switch lit.Type {
139+
case ast.LiteralInteger:
140+
// Convert positive integer to negative
141+
switch val := lit.Value.(type) {
142+
case int64:
143+
fmt.Fprintf(sb, "%sLiteral Int64_%d\n", indent, -val)
144+
return
145+
case uint64:
146+
fmt.Fprintf(sb, "%sLiteral Int64_-%d\n", indent, val)
147+
return
148+
}
149+
case ast.LiteralFloat:
150+
val := lit.Value.(float64)
151+
s := FormatFloat(-val)
152+
fmt.Fprintf(sb, "%sLiteral Float64_%s\n", indent, s)
153+
return
154+
}
155+
}
156+
}
157+
135158
fnName := UnaryOperatorToFunction(n.Op)
136159
fmt.Fprintf(sb, "%sFunction %s (children %d)\n", indent, fnName, 1)
137160
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, 1)

internal/explain/format.go

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,41 @@ import (
88
"github.com/kyleconroy/doubleclick/ast"
99
)
1010

11+
// FormatFloat formats a float value for EXPLAIN AST output
12+
func FormatFloat(val float64) string {
13+
// Use 'f' format to avoid scientific notation, -1 precision for smallest representation
14+
return strconv.FormatFloat(val, 'f', -1, 64)
15+
}
16+
17+
// escapeStringLiteral escapes special characters in a string for EXPLAIN AST output
18+
// Uses double-escaping as ClickHouse EXPLAIN AST displays strings
19+
func escapeStringLiteral(s string) string {
20+
var sb strings.Builder
21+
for _, r := range s {
22+
switch r {
23+
case '\\':
24+
sb.WriteString("\\\\\\\\") // backslash becomes four backslashes (\\\\)
25+
case '\'':
26+
sb.WriteString("\\'")
27+
case '\n':
28+
sb.WriteString("\\\\n") // newline becomes \\n
29+
case '\t':
30+
sb.WriteString("\\\\t") // tab becomes \\t
31+
case '\r':
32+
sb.WriteString("\\\\r") // carriage return becomes \\r
33+
case '\x00':
34+
sb.WriteString("\\\\0") // null becomes \\0
35+
case '\b':
36+
sb.WriteString("\\\\b") // backspace becomes \\b
37+
case '\f':
38+
sb.WriteString("\\\\f") // form feed becomes \\f
39+
default:
40+
sb.WriteRune(r)
41+
}
42+
}
43+
return sb.String()
44+
}
45+
1146
// FormatLiteral formats a literal value for EXPLAIN AST output
1247
func FormatLiteral(lit *ast.Literal) string {
1348
switch lit.Type {
@@ -26,13 +61,11 @@ func FormatLiteral(lit *ast.Literal) string {
2661
}
2762
case ast.LiteralFloat:
2863
val := lit.Value.(float64)
29-
// Use 'f' format to avoid scientific notation, -1 precision for smallest representation
30-
s := strconv.FormatFloat(val, 'f', -1, 64)
31-
return fmt.Sprintf("Float64_%s", s)
64+
return fmt.Sprintf("Float64_%s", FormatFloat(val))
3265
case ast.LiteralString:
3366
s := lit.Value.(string)
34-
// Escape backslashes in strings
35-
s = strings.ReplaceAll(s, "\\", "\\\\")
67+
// Escape special characters for display
68+
s = escapeStringLiteral(s)
3669
return fmt.Sprintf("\\'%s\\'", s)
3770
case ast.LiteralBoolean:
3871
if lit.Value.(bool) {

internal/explain/statements.go

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,18 @@ func explainCreateQuery(sb *strings.Builder, n *ast.CreateQuery, indent string,
120120
if len(n.OrderBy) == 1 {
121121
if ident, ok := n.OrderBy[0].(*ast.Identifier); ok {
122122
fmt.Fprintf(sb, "%s Identifier %s\n", indent, ident.Name())
123+
} else if lit, ok := n.OrderBy[0].(*ast.Literal); ok && lit.Type == ast.LiteralTuple {
124+
// Handle tuple literal (including empty tuple from ORDER BY ())
125+
exprs, _ := lit.Value.([]ast.Expression)
126+
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
127+
if len(exprs) > 0 {
128+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(exprs))
129+
for _, e := range exprs {
130+
Node(sb, e, depth+4)
131+
}
132+
} else {
133+
fmt.Fprintf(sb, "%s ExpressionList\n", indent)
134+
}
123135
} else {
124136
Node(sb, n.OrderBy[0], depth+2)
125137
}
@@ -135,6 +147,18 @@ func explainCreateQuery(sb *strings.Builder, n *ast.CreateQuery, indent string,
135147
if len(n.PrimaryKey) == 1 {
136148
if ident, ok := n.PrimaryKey[0].(*ast.Identifier); ok {
137149
fmt.Fprintf(sb, "%s Identifier %s\n", indent, ident.Name())
150+
} else if lit, ok := n.PrimaryKey[0].(*ast.Literal); ok && lit.Type == ast.LiteralTuple {
151+
// Handle tuple literal (including empty tuple from PRIMARY KEY ())
152+
exprs, _ := lit.Value.([]ast.Expression)
153+
fmt.Fprintf(sb, "%s Function tuple (children %d)\n", indent, 1)
154+
if len(exprs) > 0 {
155+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(exprs))
156+
for _, e := range exprs {
157+
Node(sb, e, depth+4)
158+
}
159+
} else {
160+
fmt.Fprintf(sb, "%s ExpressionList\n", indent)
161+
}
138162
} else {
139163
Node(sb, n.PrimaryKey[0], depth+2)
140164
}
@@ -156,12 +180,23 @@ func explainCreateQuery(sb *strings.Builder, n *ast.CreateQuery, indent string,
156180
}
157181
}
158182

159-
func explainDropQuery(sb *strings.Builder, n *ast.DropQuery, indent string) {
183+
func explainDropQuery(sb *strings.Builder, n *ast.DropQuery, indent string, depth int) {
160184
// DROP USER has a special output format
161185
if n.User != "" {
162186
fmt.Fprintf(sb, "%sDROP USER query\n", indent)
163187
return
164188
}
189+
190+
// Handle multiple tables: DROP TABLE t1, t2, t3
191+
if len(n.Tables) > 1 {
192+
fmt.Fprintf(sb, "%sDropQuery (children %d)\n", indent, 1)
193+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.Tables))
194+
for _, t := range n.Tables {
195+
Node(sb, t, depth+2)
196+
}
197+
return
198+
}
199+
165200
name := n.Table
166201
if n.View != "" {
167202
name = n.View

lexer/lexer.go

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -270,24 +270,62 @@ func (l *Lexer) readString(quote rune) Item {
270270

271271
for !l.eof {
272272
if l.ch == quote {
273-
// Check for escaped quote
273+
// Check for escaped quote (e.g., '' becomes ')
274274
if l.peekChar() == quote {
275-
sb.WriteRune(l.ch)
276-
l.readChar()
277-
sb.WriteRune(l.ch)
278-
l.readChar()
275+
sb.WriteRune(l.ch) // Write one quote (the escaped result)
276+
l.readChar() // skip first quote
277+
l.readChar() // skip second quote
279278
continue
280279
}
281280
l.readChar() // skip closing quote
282281
break
283282
}
284283
if l.ch == '\\' {
285-
sb.WriteRune(l.ch)
286-
l.readChar()
287-
if !l.eof {
288-
sb.WriteRune(l.ch)
284+
l.readChar() // consume backslash
285+
if l.eof {
286+
break
287+
}
288+
// Interpret escape sequence
289+
switch l.ch {
290+
case '\'':
291+
sb.WriteRune('\'')
292+
case '"':
293+
sb.WriteRune('"')
294+
case '\\':
295+
sb.WriteRune('\\')
296+
case 'n':
297+
sb.WriteRune('\n')
298+
case 't':
299+
sb.WriteRune('\t')
300+
case 'r':
301+
sb.WriteRune('\r')
302+
case '0':
303+
sb.WriteRune('\x00')
304+
case 'b':
305+
sb.WriteRune('\b')
306+
case 'f':
307+
sb.WriteRune('\f')
308+
case 'x':
309+
// Hex escape: \xNN
289310
l.readChar()
311+
if l.eof {
312+
break
313+
}
314+
hex1 := l.ch
315+
l.readChar()
316+
if l.eof {
317+
sb.WriteRune(rune(hexValue(hex1)))
318+
continue
319+
}
320+
hex2 := l.ch
321+
// Convert hex digits to byte
322+
val := hexValue(hex1)*16 + hexValue(hex2)
323+
sb.WriteByte(byte(val))
324+
default:
325+
// Unknown escape, just write the character after backslash
326+
sb.WriteRune(l.ch)
290327
}
328+
l.readChar()
291329
continue
292330
}
293331
sb.WriteRune(l.ch)
@@ -428,6 +466,19 @@ func isHexDigit(ch rune) bool {
428466
return unicode.IsDigit(ch) || (ch >= 'a' && ch <= 'f') || (ch >= 'A' && ch <= 'F')
429467
}
430468

469+
func hexValue(ch rune) int {
470+
if ch >= '0' && ch <= '9' {
471+
return int(ch - '0')
472+
}
473+
if ch >= 'a' && ch <= 'f' {
474+
return int(ch-'a') + 10
475+
}
476+
if ch >= 'A' && ch <= 'F' {
477+
return int(ch-'A') + 10
478+
}
479+
return 0
480+
}
481+
431482
func (l *Lexer) readIdentifier() Item {
432483
pos := l.pos
433484
var sb strings.Builder

0 commit comments

Comments
 (0)