Skip to content

Commit e67538d

Browse files
committed
Fix SampleRatio fraction formatting and aliased negative integers
1. SampleRatio: Convert decimal values like 0.1 to fractions like 1/10 in EXPLAIN output, matching ClickHouse's format for SAMPLE clauses. Added floatToFraction() helper to find simple fraction representation. 2. Aliased negative integers: Always output aliased negative integer literals as "Literal Int64_-N" instead of "Function negate". The previous code only did this in subquery context, but ClickHouse always uses the literal format for aliased expressions. Fixes 4 explain tests.
1 parent 1bd0d12 commit e67538d

6 files changed

Lines changed: 60 additions & 24 deletions

File tree

internal/explain/expressions.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -418,22 +418,20 @@ func explainAliasedExpr(sb *strings.Builder, n *ast.AliasedExpr, depth int) {
418418
}
419419
case *ast.UnaryExpr:
420420
// Handle negated numeric literals - output as Literal instead of Function negate
421-
// For integers, only do this in subquery context (ClickHouse behavior)
421+
// For aliased expressions, ClickHouse always shows negated integers as Literal Int64_-N
422422
// For floats (especially inf/nan), always do this
423423
if e.Op == "-" {
424424
if lit, ok := e.Operand.(*ast.Literal); ok {
425425
switch lit.Type {
426426
case ast.LiteralInteger:
427-
// Only convert to literal in subquery context
428-
if inSubqueryContext {
429-
switch val := lit.Value.(type) {
430-
case int64:
431-
fmt.Fprintf(sb, "%sLiteral Int64_%d (alias %s)\n", indent, -val, escapeAlias(n.Alias))
432-
return
433-
case uint64:
434-
fmt.Fprintf(sb, "%sLiteral Int64_-%d (alias %s)\n", indent, val, escapeAlias(n.Alias))
435-
return
436-
}
427+
// Always convert to literal for aliased expressions
428+
switch val := lit.Value.(type) {
429+
case int64:
430+
fmt.Fprintf(sb, "%sLiteral Int64_%d (alias %s)\n", indent, -val, escapeAlias(n.Alias))
431+
return
432+
case uint64:
433+
fmt.Fprintf(sb, "%sLiteral Int64_-%d (alias %s)\n", indent, val, escapeAlias(n.Alias))
434+
return
437435
}
438436
case ast.LiteralFloat:
439437
// Always convert negated floats to literals (especially for -inf, -nan)

internal/explain/tables.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,53 @@ func formatSampleRatio(sb *strings.Builder, expr ast.Expression) {
7878
formatSampleRatioOperand(sb, binExpr.Left)
7979
sb.WriteString(" / ")
8080
formatSampleRatioOperand(sb, binExpr.Right)
81+
} else if lit, ok := expr.(*ast.Literal); ok && lit.Type == ast.LiteralFloat {
82+
// Convert float to fraction if it's a simple ratio
83+
if v, ok := lit.Value.(float64); ok {
84+
num, den := floatToFraction(v)
85+
if den > 1 {
86+
fmt.Fprintf(sb, "%d / %d", num, den)
87+
return
88+
}
89+
}
90+
formatSampleRatioOperand(sb, expr)
8191
} else {
8292
formatSampleRatioOperand(sb, expr)
8393
}
8494
}
8595

96+
// floatToFraction converts a float to a simple fraction (numerator, denominator).
97+
// Returns (num, 1) if no simple fraction representation is found.
98+
func floatToFraction(f float64) (int64, int64) {
99+
// Handle common sample ratios
100+
// Try denominators from 2 to 1000
101+
for den := int64(2); den <= 1000; den++ {
102+
num := int64(f * float64(den))
103+
// Check if this gives us back the original value (within floating point tolerance)
104+
if float64(num)/float64(den) == f {
105+
// Find GCD to simplify the fraction
106+
g := gcd(num, den)
107+
return num / g, den / g
108+
}
109+
}
110+
// No simple fraction found, return as integer if possible
111+
if f == float64(int64(f)) {
112+
return int64(f), 1
113+
}
114+
return 0, 1
115+
}
116+
117+
// gcd calculates the greatest common divisor of two integers
118+
func gcd(a, b int64) int64 {
119+
if a < 0 {
120+
a = -a
121+
}
122+
for b != 0 {
123+
a, b = b, a%b
124+
}
125+
return a
126+
}
127+
86128
func formatSampleRatioOperand(sb *strings.Builder, expr ast.Expression) {
87129
if lit, ok := expr.(*ast.Literal); ok {
88130
switch v := lit.Value.(type) {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"explain_todo":{"stmt6":true}}
1+
{}

parser/testdata/00276_sample/metadata.json

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
22
"explain_todo": {
3-
"stmt13": true,
43
"stmt14": true,
54
"stmt15": true,
65
"stmt16": true,
@@ -15,7 +14,6 @@
1514
"stmt32": true,
1615
"stmt33": true,
1716
"stmt34": true,
18-
"stmt39": true,
19-
"stmt9": true
17+
"stmt39": true
2018
}
2119
}
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
{"explain_todo":{"stmt10":true,"stmt5":true,"stmt8":true,"stmt9":true}}
1+
{
2+
"explain_todo": {
3+
"stmt8": true,
4+
"stmt9": true
5+
}
6+
}
Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,9 @@
11
{
22
"explain_todo": {
3-
"stmt11": true,
43
"stmt12": true,
5-
"stmt13": true,
6-
"stmt16": true,
74
"stmt17": true,
85
"stmt18": true,
9-
"stmt19": true,
10-
"stmt22": true,
116
"stmt25": true,
12-
"stmt6": true,
13-
"stmt7": true,
14-
"stmt8": true
7+
"stmt7": true
158
}
169
}

0 commit comments

Comments
 (0)