Skip to content

Commit c983b1a

Browse files
committed
Fix WITH FILL STEP output format in ORDER BY
When ORDER BY has WITH fill step N (without FROM/TO), ClickHouse outputs the step value directly as a Literal child of OrderByElement, not wrapped in a FillModifier node. Previously we used FillModifier for step-only cases, but ClickHouse only uses FillModifier when FROM/TO contain complex expressions. Fixes 19+ statements across 6 tests related to WITH FILL.
1 parent acb8bb2 commit c983b1a

7 files changed

Lines changed: 14 additions & 22 deletions

File tree

internal/explain/select.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,9 @@ func explainOrderByElement(sb *strings.Builder, n *ast.OrderByElement, indent st
194194
hasFromOrTo := n.FillFrom != nil || n.FillTo != nil
195195
hasComplexFillExpr := hasFromOrTo && (isComplexExpr(n.FillFrom) || isComplexExpr(n.FillTo))
196196

197-
// Use FillModifier when:
198-
// 1. Only STEP is present (no FROM/TO), or
199-
// 2. FROM/TO contain complex expressions (not simple literals)
200-
useFillModifier := n.WithFill && ((n.FillStep != nil && !hasFromOrTo) || hasComplexFillExpr)
197+
// Use FillModifier when FROM/TO contain complex expressions (not simple literals)
198+
// When only STEP is present, output it directly as a child (no FillModifier)
199+
useFillModifier := n.WithFill && hasComplexFillExpr
201200

202201
if useFillModifier {
203202
// Use FillModifier wrapper

parser/testdata/00995_order_by_with_fill/metadata.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
22
"explain_todo": {
3-
"stmt16": true,
43
"stmt19": true,
54
"stmt6": true,
65
"stmt8": 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 & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt1": true,
4-
"stmt2": true
5-
}
6-
}
1+
{}
Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,8 @@
1-
{"explain_todo":{"stmt10":true,"stmt12":true,"stmt14":true,"stmt15":true,"stmt17":true,"stmt19":true,"stmt21":true,"stmt23":true,"stmt24":true,"stmt31":true,"stmt33":true,"stmt35":true,"stmt37":true,"stmt39":true,"stmt41":true,"stmt43":true,"stmt50":true,"stmt52":true,"stmt8":true}}
1+
{
2+
"explain_todo": {
3+
"stmt14": true,
4+
"stmt23": true,
5+
"stmt50": true,
6+
"stmt52": true
7+
}
8+
}

parser/testdata/02730_with_fill_by_sorting_prefix/metadata.json

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
{
22
"explain_todo": {
3-
"stmt13": true,
4-
"stmt19": true,
53
"stmt20": true,
64
"stmt21": true,
75
"stmt22": true,
@@ -14,7 +12,6 @@
1412
"stmt29": true,
1513
"stmt30": true,
1614
"stmt31": true,
17-
"stmt32": true,
18-
"stmt6": true
15+
"stmt32": true
1916
}
2017
}
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
22
"explain_todo": {
3-
"stmt1": true,
43
"stmt2": true
54
}
65
}

0 commit comments

Comments
 (0)