Skip to content

Commit 84a07ff

Browse files
committed
Fix SETTINGS placement in EXPLAIN output based on SettingsAfterFormat
When SETTINGS comes before FORMAT in a query, it should be output inside SelectQuery. When SETTINGS comes after FORMAT, it should be output at the SelectWithUnionQuery level. This fixes ~58 tests that had incorrect SETTINGS placement in their EXPLAIN AST output.
1 parent 0d6bbbe commit 84a07ff

59 files changed

Lines changed: 61 additions & 238 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: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ func explainSelectWithUnionQuery(sb *strings.Builder, n *ast.SelectWithUnionQuer
4242
break
4343
}
4444
}
45-
// When FORMAT is present, SETTINGS is output at SelectWithUnionQuery level
45+
// When SETTINGS comes AFTER FORMAT, it's output at SelectWithUnionQuery level
4646
for _, sel := range n.Selects {
47-
if sq, ok := sel.(*ast.SelectQuery); ok && sq.Format != nil && len(sq.Settings) > 0 {
47+
if sq, ok := sel.(*ast.SelectQuery); ok && sq.SettingsAfterFormat && len(sq.Settings) > 0 {
4848
fmt.Fprintf(sb, "%s Set\n", indent)
4949
break
5050
}
@@ -122,9 +122,9 @@ func explainSelectQuery(sb *strings.Builder, n *ast.SelectQuery, indent string,
122122
Node(sb, expr, depth+2)
123123
}
124124
}
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 {
125+
// SETTINGS is output at SelectQuery level only when NOT after FORMAT
126+
// When SettingsAfterFormat is true, it's output at SelectWithUnionQuery level instead
127+
if len(n.Settings) > 0 && !n.SettingsAfterFormat {
128128
fmt.Fprintf(sb, "%s Set\n", indent)
129129
}
130130
}
@@ -238,9 +238,9 @@ func countSelectUnionChildren(n *ast.SelectWithUnionQuery) int {
238238
break
239239
}
240240
}
241-
// When FORMAT is present, SETTINGS is counted at SelectWithUnionQuery level
241+
// When SETTINGS comes AFTER FORMAT, it's counted at SelectWithUnionQuery level
242242
for _, sel := range n.Selects {
243-
if sq, ok := sel.(*ast.SelectQuery); ok && sq.Format != nil && len(sq.Settings) > 0 {
243+
if sq, ok := sel.(*ast.SelectQuery); ok && sq.SettingsAfterFormat && len(sq.Settings) > 0 {
244244
count++
245245
break
246246
}
@@ -294,9 +294,8 @@ func countSelectQueryChildren(n *ast.SelectQuery) int {
294294
if n.Offset != nil {
295295
count++
296296
}
297-
// SETTINGS is counted at SelectQuery level only if there's no FORMAT
298-
// When FORMAT is present, SETTINGS is at SelectWithUnionQuery level instead
299-
if len(n.Settings) > 0 && n.Format == nil {
297+
// SETTINGS is counted at SelectQuery level only when NOT after FORMAT
298+
if len(n.Settings) > 0 && !n.SettingsAfterFormat {
300299
count++
301300
}
302301
return count
Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt3": true,
4-
"stmt5": true
5-
}
6-
}
1+
{}
Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,7 @@
11
{
22
"explain_todo": {
3-
"stmt10": true,
4-
"stmt11": true,
53
"stmt13": true,
6-
"stmt14": true,
7-
"stmt15": true,
8-
"stmt16": true,
9-
"stmt17": true,
10-
"stmt18": true,
11-
"stmt19": true,
12-
"stmt20": true,
134
"stmt22": true,
14-
"stmt23": true,
15-
"stmt24": true,
16-
"stmt25": true,
17-
"stmt26": true,
18-
"stmt27": true,
19-
"stmt28": true,
20-
"stmt29": true,
21-
"stmt4": true,
22-
"stmt5": true,
23-
"stmt6": true,
24-
"stmt7": true,
25-
"stmt8": true,
26-
"stmt9": true
5+
"stmt4": true
276
}
287
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"explain_todo":{"stmt10":true,"stmt12":true,"stmt13":true,"stmt14":true,"stmt15":true,"stmt16":true,"stmt17":true,"stmt18":true,"stmt20":true,"stmt21":true,"stmt22":true,"stmt23":true,"stmt24":true,"stmt25":true,"stmt26":true,"stmt4":true,"stmt5":true,"stmt6":true,"stmt7":true,"stmt8":true,"stmt9":true}}
1+
{}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"explain_todo":{"stmt8":true,"stmt9":true}}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt21": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt2": true
4-
}
5-
}
1+
{}

parser/testdata/01074_partial_revokes/metadata.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
"stmt34": true,
2121
"stmt35": true,
2222
"stmt36": true,
23-
"stmt37": true,
2423
"stmt39": true,
2524
"stmt4": true,
2625
"stmt40": true,
@@ -41,7 +40,6 @@
4140
"stmt59": true,
4241
"stmt6": true,
4342
"stmt60": true,
44-
"stmt61": true,
4543
"stmt63": true,
4644
"stmt64": true,
4745
"stmt66": true,
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt1": true
4-
}
5-
}
1+
{}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt4": true
4-
}
5-
}
1+
{}

0 commit comments

Comments
 (0)