Skip to content

Commit 0955c9a

Browse files
committed
Fix negative literal handling in :: cast expressions
When parsing expressions like -0::Int16, ClickHouse keeps the minus sign with the literal so it becomes CAST('-0', 'Int16'). Previously we parsed this as negate(CAST(0, Int16)). Changes: - Modified parseUnaryMinus to detect -number::type pattern and keep them together as a signed literal - Added Negative field to ast.Literal to track explicitly negative literals (needed for -0 which equals 0 numerically) - Updated formatExprAsString to handle Negative flag and output the original signed representation
1 parent 8a04ae3 commit 0955c9a

15 files changed

Lines changed: 97 additions & 101 deletions

File tree

ast/ast.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,6 +1059,7 @@ type Literal struct {
10591059
Position token.Position `json:"-"`
10601060
Type LiteralType `json:"type"`
10611061
Value interface{} `json:"value"`
1062+
Negative bool `json:"negative,omitempty"` // True if literal was explicitly negative (for -0)
10621063
}
10631064

10641065
func (l *Literal) Pos() token.Position { return l.Position }

internal/explain/format.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,10 +354,34 @@ func UnaryOperatorToFunction(op string) string {
354354
func formatExprAsString(expr ast.Expression) string {
355355
switch e := expr.(type) {
356356
case *ast.Literal:
357+
// Handle explicitly negative literals (like -0 in -0::Int16)
358+
prefix := ""
359+
if e.Negative {
360+
prefix = "-"
361+
}
357362
switch e.Type {
358363
case ast.LiteralInteger:
364+
// For explicitly negative literals, show the absolute value with prefix
365+
if e.Negative {
366+
switch v := e.Value.(type) {
367+
case int64:
368+
if v <= 0 {
369+
return fmt.Sprintf("-%d", -v)
370+
}
371+
case uint64:
372+
return fmt.Sprintf("-%d", v)
373+
}
374+
}
359375
return fmt.Sprintf("%d", e.Value)
360376
case ast.LiteralFloat:
377+
if e.Negative {
378+
switch v := e.Value.(type) {
379+
case float64:
380+
if v <= 0 {
381+
return fmt.Sprintf("%s%v", prefix, -v)
382+
}
383+
}
384+
}
361385
return fmt.Sprintf("%v", e.Value)
362386
case ast.LiteralString:
363387
return e.Value.(string)

internal/explain/functions.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ func explainCastExprWithAlias(sb *strings.Builder, n *ast.CastExpr, alias string
143143
exprStr := formatExprAsString(lit)
144144
fmt.Fprintf(sb, "%s Literal \\'%s\\'\n", indent, exprStr)
145145
}
146+
} else if negatedLit := extractNegatedLiteral(n.Expr); negatedLit != "" {
147+
// Handle negated literal like -0::Int16 -> CAST('-0', 'Int16')
148+
fmt.Fprintf(sb, "%s Literal \\'%s\\'\n", indent, negatedLit)
146149
} else {
147150
// Complex expression - use normal AST node
148151
Node(sb, n.Expr, depth+2)
@@ -333,6 +336,27 @@ func exprToLiteral(expr ast.Expression) *ast.Literal {
333336
return nil
334337
}
335338

339+
// extractNegatedLiteral checks if expr is a negated literal (like -0, -12)
340+
// and returns its string representation (like "-0", "-12") for :: cast expressions.
341+
// Returns empty string if not a negated literal.
342+
func extractNegatedLiteral(expr ast.Expression) string {
343+
unary, ok := expr.(*ast.UnaryExpr)
344+
if !ok || unary.Op != "-" {
345+
return ""
346+
}
347+
lit, ok := unary.Operand.(*ast.Literal)
348+
if !ok {
349+
return ""
350+
}
351+
switch lit.Type {
352+
case ast.LiteralInteger:
353+
return "-" + formatExprAsString(lit)
354+
case ast.LiteralFloat:
355+
return "-" + formatExprAsString(lit)
356+
}
357+
return ""
358+
}
359+
336360
func explainInExpr(sb *strings.Builder, n *ast.InExpr, indent string, depth int) {
337361
// IN is represented as Function in
338362
fnName := "in"

parser/expression.go

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -835,11 +835,50 @@ func (p *Parser) parseSpecialNumber() ast.Expression {
835835
}
836836

837837
func (p *Parser) parseUnaryMinus() ast.Expression {
838+
pos := p.current.Pos
839+
p.nextToken() // skip minus
840+
841+
// For negative number literals followed by ::, keep them together as a signed literal
842+
// This matches ClickHouse's behavior where -0::Int16 becomes CAST('-0', 'Int16')
843+
if p.currentIs(token.NUMBER) && p.peekIs(token.COLONCOLON) {
844+
// Parse the number and create a "signed" literal
845+
// We'll store the negative sign in the raw value
846+
numVal := "-" + p.current.Value
847+
lit := &ast.Literal{
848+
Position: pos,
849+
Type: ast.LiteralInteger,
850+
Negative: true, // Mark as explicitly negative for proper formatting
851+
}
852+
// Check if it's a float
853+
if strings.Contains(numVal, ".") || strings.ContainsAny(numVal, "eE") {
854+
f, _ := strconv.ParseFloat(numVal, 64)
855+
lit.Type = ast.LiteralFloat
856+
lit.Value = f
857+
} else {
858+
i, _ := strconv.ParseInt(numVal, 10, 64)
859+
lit.Value = i
860+
}
861+
p.nextToken() // move past number
862+
// Apply postfix operators like :: using the expression parsing loop
863+
left := ast.Expression(lit)
864+
for !p.currentIs(token.EOF) && LOWEST < p.precedenceForCurrent() {
865+
startPos := p.current.Pos
866+
left = p.parseInfixExpression(left)
867+
if left == nil {
868+
return nil
869+
}
870+
if p.current.Pos == startPos {
871+
break
872+
}
873+
}
874+
return left
875+
}
876+
877+
// Standard unary minus handling
838878
expr := &ast.UnaryExpr{
839-
Position: p.current.Pos,
879+
Position: pos,
840880
Op: "-",
841881
}
842-
p.nextToken()
843882
expr.Operand = p.parseExpression(UNARY)
844883
return expr
845884
}
Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1 @@
1-
{
2-
"explain_todo": {
3-
"stmt1": true,
4-
"stmt2": true,
5-
"stmt3": true,
6-
"stmt4": true,
7-
"stmt5": true,
8-
"stmt6": true
9-
}
10-
}
1+
{}
Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
22
"explain_todo": {
3-
"stmt5": true,
4-
"stmt7": true
3+
"stmt5": true
54
}
65
}
Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
{
22
"explain_todo": {
3-
"stmt12": true,
4-
"stmt13": true,
5-
"stmt14": true,
6-
"stmt15": true,
7-
"stmt16": true,
8-
"stmt17": true,
9-
"stmt18": true,
10-
"stmt19": true
3+
"stmt14": true
114
}
125
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"explain_todo":{"stmt11":true,"stmt12":true,"stmt13":true}}
1+
{}

parser/testdata/02685_decimal256_various/metadata.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
"explain_todo": {
33
"stmt10": true,
44
"stmt27": true,
5-
"stmt28": true,
6-
"stmt29": true,
75
"stmt30": true,
86
"stmt31": true,
97
"stmt32": true,

parser/testdata/02708_dotProduct/metadata.json

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@
1212
"stmt19": true,
1313
"stmt20": true,
1414
"stmt21": true,
15-
"stmt27": true,
16-
"stmt28": true,
17-
"stmt29": true,
18-
"stmt30": true,
1915
"stmt34": true,
2016
"stmt35": true,
2117
"stmt36": true,

0 commit comments

Comments
 (0)