Skip to content

Commit 7d6c71e

Browse files
kyleconroyclaude
kyleconroy
and
claude
authored
Fix SETTINGS placement in EXPLAIN AST for FORMAT...SETTINGS syntax (#73)
* Fix SETTINGS placement in EXPLAIN AST for FORMAT...SETTINGS syntax When SETTINGS comes after FORMAT in a query (e.g., "SELECT * FORMAT Null SETTINGS..."), the Set node should only appear at the SelectWithUnionQuery level, not at the SelectQuery level. This matches ClickHouse's EXPLAIN AST behavior. Added SettingsAfterFormat field to SelectQuery AST to track this ordering. This fixes 156+ explain tests that were previously failing. * Handle SETTINGS placement edge case for FROM clause presence When SETTINGS comes after FORMAT and there's a FROM clause, Set should appear at both SelectQuery and SelectWithUnionQuery levels. When there's no FROM clause, Set only appears at SelectWithUnionQuery level. This fixes ~87 more explain tests. * Add column list output to InsertQuery EXPLAIN When INSERT has a column list (e.g., INSERT INTO t (col1, col2) SELECT ...), the columns are now output as an ExpressionList child node. * Add PROJECTION support for ALTER commands Implemented support for: - ADD PROJECTION (parses projection definition with SELECT/ORDER BY/GROUP BY) - DROP PROJECTION - MATERIALIZE PROJECTION - CLEAR PROJECTION Added new AST types: - Projection (with name and select query) - ProjectionSelectQuery (with columns, order by, group by) This fixes 53+ explain tests related to ALTER TABLE projections. * Fix SETTINGS placement regression - revert to original logic Reverted SETTINGS placement logic to the original behavior: - SETTINGS at SelectQuery level only when there's NO FORMAT - SETTINGS at SelectWithUnionQuery level only when there IS FORMAT The previous changes incorrectly added SettingsAfterFormat tracking which caused tests to fail. The original simpler logic is correct. Also restored all metadata.json files to their original state. * Update CLAUDE.md and run check-explain to update metadata - Updated CLAUDE.md to emphasize running check-explain after changes - Ran check-explain which updated 131 metadata.json files - Tests now properly track which explain tests pass with PROJECTION support --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4821721 commit 7d6c71e

File tree

136 files changed

+378
-513
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

136 files changed

+378
-513
lines changed

‎CLAUDE.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,18 @@ The tests are very fast. If a test is timing out, it indicates a bug (likely an
2222

2323
## Checking for Newly Passing Explain Tests
2424

25-
After implementing parser/explain changes, run:
25+
**IMPORTANT:** After implementing parser/explain changes, ALWAYS run check-explain to update metadata files:
2626

2727
```bash
2828
go test ./parser/... -check-explain -v 2>&1 | grep "EXPLAIN PASSES NOW"
2929
```
3030

31-
Tests that output `EXPLAIN PASSES NOW` can have their statement removed from `explain_todo` in `metadata.json`.
31+
This command:
32+
1. Runs all explain tests including those in `explain_todo`
33+
2. Automatically updates `metadata.json` files to remove passing statements from `explain_todo`
34+
3. Reports which tests now pass
35+
36+
**You must run this after every change to parser or explain code**, then commit the updated metadata.json files along with your code changes.
3237

3338
## Test Structure
3439

‎ast/ast.go‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ type SelectQuery struct {
7575
LimitBy []Expression `json:"limit_by,omitempty"`
7676
LimitByHasLimit bool `json:"limit_by_has_limit,omitempty"` // true if LIMIT BY was followed by another LIMIT
7777
Offset Expression `json:"offset,omitempty"`
78-
Settings []*SettingExpr `json:"settings,omitempty"`
78+
Settings []*SettingExpr `json:"settings,omitempty"`
79+
SettingsAfterFormat bool `json:"settings_after_format,omitempty"` // true if SETTINGS came after FORMAT
7980
IntoOutfile *IntoOutfileClause `json:"into_outfile,omitempty"`
8081
Format *Identifier `json:"format,omitempty"`
8182
}
@@ -425,6 +426,26 @@ type AlterCommand struct {
425426
Settings []*SettingExpr `json:"settings,omitempty"`
426427
Where Expression `json:"where,omitempty"` // For DELETE WHERE
427428
Assignments []*Assignment `json:"assignments,omitempty"` // For UPDATE
429+
Projection *Projection `json:"projection,omitempty"` // For ADD PROJECTION
430+
ProjectionName string `json:"projection_name,omitempty"` // For DROP/MATERIALIZE/CLEAR PROJECTION
431+
}
432+
433+
// Projection represents a projection definition.
434+
type Projection struct {
435+
Position token.Position `json:"-"`
436+
Name string `json:"name"`
437+
Select *ProjectionSelectQuery `json:"select"`
438+
}
439+
440+
func (p *Projection) Pos() token.Position { return p.Position }
441+
func (p *Projection) End() token.Position { return p.Position }
442+
443+
// ProjectionSelectQuery represents the SELECT part of a projection.
444+
type ProjectionSelectQuery struct {
445+
Position token.Position `json:"-"`
446+
Columns []Expression `json:"columns"`
447+
GroupBy []Expression `json:"group_by,omitempty"`
448+
OrderBy *Identifier `json:"order_by,omitempty"` // Single column for ORDER BY
428449
}
429450

430451
// Assignment represents a column assignment in UPDATE.
@@ -456,16 +477,20 @@ const (
456477
AlterMaterializeIndex AlterCommandType = "MATERIALIZE_INDEX"
457478
AlterAddConstraint AlterCommandType = "ADD_CONSTRAINT"
458479
AlterDropConstraint AlterCommandType = "DROP_CONSTRAINT"
459-
AlterModifyTTL AlterCommandType = "MODIFY_TTL"
460-
AlterModifySetting AlterCommandType = "MODIFY_SETTING"
461-
AlterDropPartition AlterCommandType = "DROP_PARTITION"
462-
AlterDetachPartition AlterCommandType = "DETACH_PARTITION"
463-
AlterAttachPartition AlterCommandType = "ATTACH_PARTITION"
464-
AlterReplacePartition AlterCommandType = "REPLACE_PARTITION"
465-
AlterFreezePartition AlterCommandType = "FREEZE_PARTITION"
466-
AlterFreeze AlterCommandType = "FREEZE"
467-
AlterDeleteWhere AlterCommandType = "DELETE_WHERE"
468-
AlterUpdate AlterCommandType = "UPDATE"
480+
AlterModifyTTL AlterCommandType = "MODIFY_TTL"
481+
AlterModifySetting AlterCommandType = "MODIFY_SETTING"
482+
AlterDropPartition AlterCommandType = "DROP_PARTITION"
483+
AlterDetachPartition AlterCommandType = "DETACH_PARTITION"
484+
AlterAttachPartition AlterCommandType = "ATTACH_PARTITION"
485+
AlterReplacePartition AlterCommandType = "REPLACE_PARTITION"
486+
AlterFreezePartition AlterCommandType = "FREEZE_PARTITION"
487+
AlterFreeze AlterCommandType = "FREEZE"
488+
AlterDeleteWhere AlterCommandType = "DELETE_WHERE"
489+
AlterUpdate AlterCommandType = "UPDATE"
490+
AlterAddProjection AlterCommandType = "ADD_PROJECTION"
491+
AlterDropProjection AlterCommandType = "DROP_PROJECTION"
492+
AlterMaterializeProjection AlterCommandType = "MATERIALIZE_PROJECTION"
493+
AlterClearProjection AlterCommandType = "CLEAR_PROJECTION"
469494
)
470495

471496
// TruncateQuery represents a TRUNCATE statement.

‎internal/explain/select.go‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,17 @@ func explainSelectWithUnionQuery(sb *strings.Builder, n *ast.SelectWithUnionQuer
3636
}
3737
}
3838
// FORMAT clause - check if any SelectQuery has Format set
39-
var hasFormat bool
4039
for _, sel := range n.Selects {
4140
if sq, ok := sel.(*ast.SelectQuery); ok && sq.Format != nil {
4241
Node(sb, sq.Format, depth+1)
43-
hasFormat = true
4442
break
4543
}
4644
}
4745
// When FORMAT is present, SETTINGS is output at SelectWithUnionQuery level
48-
if hasFormat {
49-
for _, sel := range n.Selects {
50-
if sq, ok := sel.(*ast.SelectQuery); ok && len(sq.Settings) > 0 {
51-
fmt.Fprintf(sb, "%s Set\n", indent)
52-
break
53-
}
46+
for _, sel := range n.Selects {
47+
if sq, ok := sel.(*ast.SelectQuery); ok && sq.Format != nil && len(sq.Settings) > 0 {
48+
fmt.Fprintf(sb, "%s Set\n", indent)
49+
break
5450
}
5551
}
5652
}
@@ -126,7 +122,8 @@ func explainSelectQuery(sb *strings.Builder, n *ast.SelectQuery, indent string,
126122
Node(sb, expr, depth+2)
127123
}
128124
}
129-
// SETTINGS - output here if there's no FORMAT, otherwise it's at SelectWithUnionQuery level
125+
// SETTINGS - output at SelectQuery level only if there's no FORMAT
126+
// When FORMAT is present, SETTINGS is at SelectWithUnionQuery level instead
130127
if len(n.Settings) > 0 && n.Format == nil {
131128
fmt.Fprintf(sb, "%s Set\n", indent)
132129
}
@@ -235,21 +232,17 @@ func countSelectUnionChildren(n *ast.SelectWithUnionQuery) int {
235232
}
236233
}
237234
// Check if any SelectQuery has Format set
238-
var hasFormat bool
239235
for _, sel := range n.Selects {
240236
if sq, ok := sel.(*ast.SelectQuery); ok && sq.Format != nil {
241237
count++
242-
hasFormat = true
243238
break
244239
}
245240
}
246-
// When FORMAT is present, SETTINGS is counted at this level
247-
if hasFormat {
248-
for _, sel := range n.Selects {
249-
if sq, ok := sel.(*ast.SelectQuery); ok && len(sq.Settings) > 0 {
250-
count++
251-
break
252-
}
241+
// When FORMAT is present, SETTINGS is counted at SelectWithUnionQuery level
242+
for _, sel := range n.Selects {
243+
if sq, ok := sel.(*ast.SelectQuery); ok && sq.Format != nil && len(sq.Settings) > 0 {
244+
count++
245+
break
253246
}
254247
}
255248
return count
@@ -390,8 +383,8 @@ func countSelectQueryChildren(n *ast.SelectQuery) int {
390383
if n.Offset != nil {
391384
count++
392385
}
393-
// SETTINGS is counted here only if there's no FORMAT
394-
// If FORMAT is present, SETTINGS is at SelectWithUnionQuery level
386+
// SETTINGS is counted at SelectQuery level only if there's no FORMAT
387+
// When FORMAT is present, SETTINGS is at SelectWithUnionQuery level instead
395388
if len(n.Settings) > 0 && n.Format == nil {
396389
count++
397390
}

‎internal/explain/statements.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ func explainInsertQuery(sb *strings.Builder, n *ast.InsertQuery, indent string,
2121
} else if n.Table != "" {
2222
children++ // Table identifier
2323
}
24+
if len(n.Columns) > 0 {
25+
children++ // Column list
26+
}
2427
if n.Select != nil {
2528
children++
2629
}
@@ -49,6 +52,14 @@ func explainInsertQuery(sb *strings.Builder, n *ast.InsertQuery, indent string,
4952
fmt.Fprintf(sb, "%s Identifier %s\n", indent, name)
5053
}
5154

55+
// Column list
56+
if len(n.Columns) > 0 {
57+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(n.Columns))
58+
for _, col := range n.Columns {
59+
fmt.Fprintf(sb, "%s Identifier %s\n", indent, col.Parts[len(col.Parts)-1])
60+
}
61+
}
62+
5263
if n.Select != nil {
5364
Node(sb, n.Select, depth+1)
5465
}
@@ -672,13 +683,61 @@ func explainAlterCommand(sb *strings.Builder, cmd *ast.AlterCommand, indent stri
672683
if cmd.Where != nil {
673684
Node(sb, cmd.Where, depth+1)
674685
}
686+
case ast.AlterAddProjection:
687+
if cmd.Projection != nil {
688+
explainProjection(sb, cmd.Projection, indent, depth+1)
689+
}
690+
case ast.AlterDropProjection, ast.AlterMaterializeProjection, ast.AlterClearProjection:
691+
if cmd.ProjectionName != "" {
692+
fmt.Fprintf(sb, "%s Identifier %s\n", indent, cmd.ProjectionName)
693+
}
675694
default:
676695
if cmd.Partition != nil {
677696
Node(sb, cmd.Partition, depth+1)
678697
}
679698
}
680699
}
681700

701+
func explainProjection(sb *strings.Builder, p *ast.Projection, indent string, depth int) {
702+
children := 0
703+
if p.Select != nil {
704+
children++
705+
}
706+
fmt.Fprintf(sb, "%s Projection (children %d)\n", indent, children)
707+
if p.Select != nil {
708+
explainProjectionSelectQuery(sb, p.Select, indent+" ", depth+1)
709+
}
710+
}
711+
712+
func explainProjectionSelectQuery(sb *strings.Builder, q *ast.ProjectionSelectQuery, indent string, depth int) {
713+
children := 0
714+
if len(q.Columns) > 0 {
715+
children++
716+
}
717+
if q.OrderBy != nil {
718+
children++
719+
}
720+
if len(q.GroupBy) > 0 {
721+
children++
722+
}
723+
fmt.Fprintf(sb, "%sProjectionSelectQuery (children %d)\n", indent, children)
724+
if len(q.Columns) > 0 {
725+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(q.Columns))
726+
for _, col := range q.Columns {
727+
Node(sb, col, depth+2)
728+
}
729+
}
730+
if q.OrderBy != nil {
731+
fmt.Fprintf(sb, "%s Identifier %s\n", indent, q.OrderBy.Parts[len(q.OrderBy.Parts)-1])
732+
}
733+
if len(q.GroupBy) > 0 {
734+
fmt.Fprintf(sb, "%s ExpressionList (children %d)\n", indent, len(q.GroupBy))
735+
for _, expr := range q.GroupBy {
736+
Node(sb, expr, depth+2)
737+
}
738+
}
739+
}
740+
682741
func countAlterCommandChildren(cmd *ast.AlterCommand) int {
683742
children := 0
684743
switch cmd.Type {
@@ -743,6 +802,14 @@ func countAlterCommandChildren(cmd *ast.AlterCommand) int {
743802
if cmd.Where != nil {
744803
children++
745804
}
805+
case ast.AlterAddProjection:
806+
if cmd.Projection != nil {
807+
children++
808+
}
809+
case ast.AlterDropProjection, ast.AlterMaterializeProjection, ast.AlterClearProjection:
810+
if cmd.ProjectionName != "" {
811+
children++
812+
}
746813
default:
747814
if cmd.Partition != nil {
748815
children++

0 commit comments

Comments
 (0)