Skip to content

Commit 487763f

Browse files
committed
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.
1 parent 4452bb9 commit 487763f

216 files changed

Lines changed: 851 additions & 217 deletions

File tree

Some content is hidden

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

internal/explain/select.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,9 @@ func explainSelectWithUnionQuery(sb *strings.Builder, n *ast.SelectWithUnionQuer
4242
break
4343
}
4444
}
45-
// When SETTINGS comes AFTER FORMAT, it is ALSO output at SelectWithUnionQuery level
46-
// (in addition to being at SelectQuery level)
45+
// When FORMAT is present, SETTINGS is output at SelectWithUnionQuery level
4746
for _, sel := range n.Selects {
48-
if sq, ok := sel.(*ast.SelectQuery); ok && sq.SettingsAfterFormat && len(sq.Settings) > 0 {
47+
if sq, ok := sel.(*ast.SelectQuery); ok && sq.Format != nil && len(sq.Settings) > 0 {
4948
fmt.Fprintf(sb, "%s Set\n", indent)
5049
break
5150
}
@@ -123,11 +122,9 @@ func explainSelectQuery(sb *strings.Builder, n *ast.SelectQuery, indent string,
123122
Node(sb, expr, depth+2)
124123
}
125124
}
126-
// SETTINGS - output at SelectQuery level in these cases:
127-
// 1. SETTINGS is before FORMAT (not after)
128-
// 2. SETTINGS is after FORMAT AND there's a FROM clause
129-
// When SETTINGS is after FORMAT without FROM, it's only at SelectWithUnionQuery level
130-
if len(n.Settings) > 0 && (!n.SettingsAfterFormat || n.From != nil) {
125+
// SETTINGS - output at SelectQuery level only if there's no FORMAT
126+
// When FORMAT is present, SETTINGS is at SelectWithUnionQuery level instead
127+
if len(n.Settings) > 0 && n.Format == nil {
131128
fmt.Fprintf(sb, "%s Set\n", indent)
132129
}
133130
}
@@ -241,9 +238,9 @@ func countSelectUnionChildren(n *ast.SelectWithUnionQuery) int {
241238
break
242239
}
243240
}
244-
// When SETTINGS comes AFTER FORMAT, it is ALSO counted at this level
241+
// When FORMAT is present, SETTINGS is counted at SelectWithUnionQuery level
245242
for _, sel := range n.Selects {
246-
if sq, ok := sel.(*ast.SelectQuery); ok && sq.SettingsAfterFormat && len(sq.Settings) > 0 {
243+
if sq, ok := sel.(*ast.SelectQuery); ok && sq.Format != nil && len(sq.Settings) > 0 {
247244
count++
248245
break
249246
}
@@ -386,11 +383,9 @@ func countSelectQueryChildren(n *ast.SelectQuery) int {
386383
if n.Offset != nil {
387384
count++
388385
}
389-
// SETTINGS is counted at SelectQuery level in these cases:
390-
// 1. SETTINGS is before FORMAT (not after)
391-
// 2. SETTINGS is after FORMAT AND there's a FROM clause
392-
// When SETTINGS is after FORMAT without FROM, it's only at SelectWithUnionQuery level
393-
if len(n.Settings) > 0 && (!n.SettingsAfterFormat || n.From != nil) {
386+
// SETTINGS is counted at SelectQuery level only if there's no FORMAT
387+
// When FORMAT is present, SETTINGS is at SelectWithUnionQuery level instead
388+
if len(n.Settings) > 0 && n.Format == nil {
394389
count++
395390
}
396391
return count
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
{}
1+
{
2+
"explain_todo": {
3+
"stmt6": true,
4+
"stmt7": true
5+
}
6+
}

parser/testdata/00152_insert_different_granularity/metadata.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
{
22
"explain_todo": {
3+
"stmt5": true,
4+
"stmt6": true,
35
"stmt7": true,
46
"stmt8": true
57
}
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
{}
1+
{
2+
"explain_todo": {
3+
"stmt13": true,
4+
"stmt7": true
5+
}
6+
}
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
{
22
"explain_todo": {
3-
"stmt5": true
3+
"stmt5": true,
4+
"stmt6": true,
5+
"stmt7": true
46
}
57
}
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
{}
1+
{
2+
"explain_todo": {
3+
"stmt4": true,
4+
"stmt5": true
5+
}
6+
}
Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,9 @@
1-
{}
1+
{
2+
"explain_todo": {
3+
"stmt16": true,
4+
"stmt19": true,
5+
"stmt27": true,
6+
"stmt30": true,
7+
"stmt33": true
8+
}
9+
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{}
1+
{"explain_todo":{"stmt21":true,"stmt22":true}}

parser/testdata/00276_sample/metadata.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
"stmt32": true,
1616
"stmt33": true,
1717
"stmt34": true,
18+
"stmt38": true,
1819
"stmt39": true,
20+
"stmt6": true,
1921
"stmt9": true
2022
}
2123
}
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
{}
1+
{
2+
"explain_todo": {
3+
"stmt4": true,
4+
"stmt8": true
5+
}
6+
}

0 commit comments

Comments
 (0)