From e5b396c64e54aa54cfd857955f2e9d4b88f1ed37 Mon Sep 17 00:00:00 2001 From: Wei Cao Date: Mon, 24 Aug 2026 00:03:05 +0800 Subject: [PATCH] fix: preserve FORMAT numeric precision Preserve integer and DECIMAL precision while translating MySQL FORMAT. Generated through the GitHub platform commit API. --- format_precision_test.go | 32 ++++++++ transpiler/translate.go | 142 +++++++++++++++++++++++++++++++---- transpiler/translate_test.go | 28 +++++-- 3 files changed, 182 insertions(+), 20 deletions(-) create mode 100644 format_precision_test.go diff --git a/format_precision_test.go b/format_precision_test.go new file mode 100644 index 0000000..36e7645 --- /dev/null +++ b/format_precision_test.go @@ -0,0 +1,32 @@ +package main + +import ( + "testing" + + "github.com/dolthub/go-mysql-server/enginetest" + "github.com/dolthub/go-mysql-server/enginetest/queries" + "github.com/dolthub/go-mysql-server/enginetest/scriptgen/setup" + "github.com/dolthub/go-mysql-server/sql" + "github.com/stretchr/testify/require" +) + +func TestFormatPreservesNumericPrecision(t *testing.T) { + h := NewDefaultDuckHarness() + h.Setup(setup.MydbData, setup.MytableData) + e, err := h.NewEngine(t) + require.NoError(t, err) + defer e.Close() + + for _, test := range []queries.QueryTest{ + { + Query: "SELECT FORMAT(9007199254740993, 0) FROM mytable LIMIT 1", + Expected: []sql.Row{{"9,007,199,254,740,993"}}, + }, + { + Query: "SELECT FORMAT(CAST('1234567890123456789012345678.12' AS DECIMAL(30, 2)), 2) FROM mytable LIMIT 1", + Expected: []sql.Row{{"1,234,567,890,123,456,789,012,345,678.12"}}, + }, + } { + enginetest.TestQueryWithEngine(t, h, e, test) + } +} diff --git a/transpiler/translate.go b/transpiler/translate.go index 54877c2..1721956 100644 --- a/transpiler/translate.go +++ b/transpiler/translate.go @@ -630,30 +630,146 @@ def rewrite_mysql_for_duckdb(node): # MySQL FORMAT(x, d[, locale]) is number-to-string with grouping. # DuckDB FORMAT is {fmt}; map the scale and swap separators for common EU locales. if isinstance(node, exp.NumberToStr): + # DuckDB rejects precision specifiers for integer values. Keep the format + # string dynamic so DuckDB can select its integer form at execution time; + # casting every value to DOUBLE would silently lose large-number precision. value = node.this + integer_types = [ + exp.Literal.string(t) + for t in ( + "TINYINT", "SMALLINT", "INTEGER", "BIGINT", "HUGEINT", + "UTINYINT", "USMALLINT", "UINTEGER", "UBIGINT", + ) + ] + string_types = [ + exp.Literal.string(t) + for t in ("VARCHAR", "CHAR", "BPCHAR", "TEXT", "STRING") + ] + value_type = exp.Anonymous(this="typeof", expressions=[value.copy()]) + is_integer = exp.In(this=value_type.copy(), expressions=integer_types) + is_decimal = exp.Like(this=value_type.copy(), expression=exp.Literal.string("DECIMAL%%")) + is_string = exp.In(this=value_type, expressions=string_types) digits = node.args.get("format") culture = node.args.get("culture") if isinstance(digits, exp.Literal) and digits.is_int: fmt = "{:,." + str(int(digits.this)) + "f}" - formatted = exp.Anonymous( - this="format", - expressions=[exp.Literal.string(fmt), value], - ) + format_expr = exp.Literal.string(fmt) + integer_suffix = None + if int(digits.this) > 0: + integer_suffix = exp.Anonymous( + this="concat", + expressions=[ + exp.Literal.string("."), + exp.Anonymous( + this="repeat", + expressions=[exp.Literal.string("0"), digits.copy()], + ), + ], + ) else: - formatted = exp.Anonymous( - this="format", + format_expr = exp.Anonymous( + this="concat", + expressions=[ + exp.Literal.string("{:,."), + exp.Cast(this=digits, to=exp.DataType.build("TEXT")), + exp.Literal.string("f}"), + ], + ) + integer_suffix = exp.Case( + ifs=[ + exp.If( + this=exp.And( + this=is_integer.copy(), + expression=exp.GT(this=digits.copy(), expression=exp.Literal.number(0)), + ), + true=exp.Anonymous( + this="concat", + expressions=[ + exp.Literal.string("."), + exp.Anonymous( + this="repeat", + expressions=[exp.Literal.string("0"), digits.copy()], + ), + ], + ), + ) + ], + default=exp.Literal.string(""), + ) + numeric_format_expr = exp.Case( + ifs=[exp.If(this=is_integer.copy(), true=exp.Literal.string("{:,}"))], + default=format_expr, + ) + numeric_formatted = exp.Anonymous( + this="format", + expressions=[numeric_format_expr, value.copy()], + ) + string_formatted = exp.Anonymous( + this="format", + expressions=[ + format_expr.copy(), + exp.TryCast(this=value.copy(), to=exp.DataType.build("DOUBLE")), + ], + ) + rounded_decimal = exp.Cast( + this=exp.Anonymous( + this="round", + expressions=[value.copy(), digits.copy()], + ), + to=exp.DataType.build("TEXT"), + ) + decimal_integer = exp.Anonymous( + this="split_part", + expressions=[rounded_decimal.copy(), exp.Literal.string("."), exp.Literal.number(1)], + ) + if isinstance(digits, exp.Literal) and digits.is_int and int(digits.this) > 0: + decimal_fraction = exp.Anonymous( + this="rpad", expressions=[ exp.Anonymous( - this="concat", - expressions=[ - exp.Literal.string("{:,."), - exp.Cast(this=digits, to=exp.DataType.build("TEXT")), - exp.Literal.string("f}"), - ], + this="split_part", + expressions=[rounded_decimal.copy(), exp.Literal.string("."), exp.Literal.number(2)], ), - value, + digits.copy(), + exp.Literal.string("0"), ], ) + decimal_text = exp.Anonymous( + this="concat", + expressions=[decimal_integer, exp.Literal.string("."), decimal_fraction], + ) + else: + decimal_text = decimal_integer + # DuckDB FORMAT currently routes DECIMAL through a floating formatter. + # Group the rounded TEXT representation instead, which keeps up to the + # full DECIMAL precision supported by DuckDB. + decimal_formatted = decimal_text + for grouping_pass in range(12): + pattern = r"(\d+)(\d{3})([.]|$)" if grouping_pass == 0 else r"(\d+)(\d{3})([,.]|$)" + decimal_formatted = exp.Anonymous( + this="regexp_replace", + expressions=[ + decimal_formatted, + exp.Literal.string(pattern), + exp.Literal.string(r"\1,\2\3"), + exp.Literal.string("g"), + ], + ) + formatted = exp.Case( + ifs=[ + exp.If(this=is_string, true=string_formatted), + exp.If(this=is_decimal, true=decimal_formatted), + ], + default=numeric_formatted, + ) + if integer_suffix is not None: + formatted = exp.Anonymous( + this="concat", + expressions=[formatted, exp.Case( + ifs=[exp.If(this=is_integer, true=integer_suffix)], + default=exp.Literal.string(""), + )], + ) if isinstance(culture, exp.Literal) and culture.is_string: loc = str(culture.this).lower().replace("-", "_") prefix = loc.split("_", 1)[0] diff --git a/transpiler/translate_test.go b/transpiler/translate_test.go index 9e84f1f..7988ecd 100644 --- a/transpiler/translate_test.go +++ b/transpiler/translate_test.go @@ -14,6 +14,7 @@ func TestTranslate(t *testing.T) { name string input string expected string + validate func(*testing.T, string) }{ { @@ -82,14 +83,25 @@ func TestTranslate(t *testing.T) { expected: "SELECT FLOOR(i), AVG(LENGTH(s)) FROM mytable AS mt GROUP BY 1 ORDER BY FLOOR(i) DESC", }, { - name: "FORMAT two-arg uses DuckDB fmt grouping", - input: "SELECT FORMAT(i, 3) FROM mytable", - expected: "SELECT FORMAT('{:,.3f}', i) FROM mytable", + name: "FORMAT two-arg uses DuckDB fmt grouping", + input: "SELECT FORMAT(i, 3) FROM mytable", + validate: func(t *testing.T, result string) { + if !strings.Contains(result, "TYPEOF(i)") || !strings.Contains(result, "THEN '{:,}'") { + t.Errorf("FORMAT translation must select an integer-safe format dynamically: %s", result) + } + if strings.Contains(result, "FORMAT('{:,.3f}', CAST(i AS DOUBLE))") { + t.Errorf("FORMAT translation must not coerce the value to DOUBLE: %s", result) + } + }, }, { - name: "FORMAT with da_DK swaps grouping separators", - input: "SELECT FORMAT(i, 3, 'da_DK') FROM mytable", - expected: "SELECT REPLACE(REPLACE(REPLACE(FORMAT('{:,.3f}', i), ',', '\x01'), '.', ','), '\x01', '.') FROM mytable", + name: "FORMAT with da_DK swaps grouping separators", + input: "SELECT FORMAT(i, 3, 'da_DK') FROM mytable", + validate: func(t *testing.T, result string) { + if !strings.Contains(result, "REPLACE(REPLACE(REPLACE(") || !strings.Contains(result, "TYPEOF(i)") { + t.Errorf("FORMAT locale translation lost the precision-safe formatter: %s", result) + } + }, }, { name: "aggregate without GROUP BY wraps non-agg columns", @@ -618,7 +630,9 @@ func TestTranslate(t *testing.T) { trimmedResult := strings.TrimSpace(result) fmt.Println("trimmedResult:", trimmedResult) - if trimmedResult != tc.expected { + if tc.validate != nil { + tc.validate(t, trimmedResult) + } else if trimmedResult != tc.expected { t.Errorf("translate(%q) = %v; want %v", tc.input, trimmedResult, tc.expected) } })