Skip to content

Commit 390711a

Browse files
committed
fix(go/adbc/driver/snowflake): Use Decimal128 for NUMBER with scale>0 when use_high_precision=false
Previously, when use_high_precision=false, NUMBER columns with scale>0 were returned as scaled Int64 from Snowflake. This mismatch caused data corruption at the clients with the decimal data showing up as Int64. The fix changes the behavior to use Decimal128 for all non-integer NUMBER types (scale>0) even when use_high_precision=false, ensuring: - Type consistency between schema and data - Exact precision preservation (no floating-point approximation) - The Int64 optimization for NUMBER(p,0) is preserved Changes: - record_reader.go: Use Decimal128 for NUMBER(p,s>0) when use_high_precision=false - connection.go: Update schema inference to match record_reader logic - driver_test.go: Add comprehensive tests for NUMBER type handling - snowflake.rst: Update documentation to reflect new behavior This is a different issue from apache#1242 (fixed in PR apache#1267), which addressed the Int64→Decimal128 conversion for use_high_precision=true. This fix addresses the type mismatch in the use_high_precision=false code path. Breaking change: Applications expecting Float64 for NUMBER(p,s>0) with use_high_precision=false will now receive Decimal128. While this is a breaking change, the previous functionality was returning incorrect values (as scaled Int64) to the client. The documentation is changed accordingly. I don't think returning decimal data as float is right since float/double are in seperate category. This is per obervation by @lidavidm at apache#3295
1 parent 7687b15 commit 390711a

File tree

4 files changed

+255
-69
lines changed

4 files changed

+255
-69
lines changed

docs/source/driver/snowflake.rst

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -469,11 +469,11 @@ These options map 1:1 with the Snowflake `Config object <https://pkg.go.dev/gith
469469
to ``true`` on Windows/OSX, ``false`` on Linux.
470470

471471
``adbc.snowflake.sql.client_option.use_high_precision``
472-
When ``true``, fixed-point snowflake columns with the type ``NUMBER``
473-
will be returned as ``Decimal128`` type Arrow columns using the precision
474-
and scale of the ``NUMBER`` type. When ``false``, ``NUMBER`` columns
475-
with a scale of 0 will be returned as ``Int64`` typed Arrow columns and
476-
non-zero scaled columns will be returned as ``Float64`` typed Arrow columns.
472+
When ``true``, all Snowflake ``NUMBER`` columns will be returned as
473+
``Decimal128`` type Arrow columns preserving the exact precision and scale.
474+
When ``false``, ``NUMBER`` columns with a scale of 0 will be returned as
475+
``Int64`` typed Arrow columns for better performance, while non-zero scaled
476+
columns will still be returned as ``Decimal128`` to preserve precision.
477477
The default is ``true``.
478478

479479
``adbc.snowflake.sql.client_option.max_timestamp_precision``
@@ -543,9 +543,10 @@ indicated are done to ensure consistency of the stream of record batches.
543543

544544
* - decimal/numeric
545545
- numeric
546-
- Snowflake will respect the precision/scale of the Arrow type. See the
547-
``adbc.snowflake.sql.client_option.use_high_precision`` for exceptions to this
548-
behavior.
546+
- Snowflake will respect the precision/scale of the Arrow type. When reading,
547+
the ``adbc.snowflake.sql.client_option.use_high_precision`` option controls
548+
the Arrow type used: ``true`` returns all as Decimal128, ``false`` returns
549+
integers (scale=0) as Int64 and decimals (scale>0) as Decimal128.
549550

550551
* - time
551552
- time64[ns]

go/adbc/driver/snowflake/connection.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -442,16 +442,15 @@ func (c *connectionImpl) toArrowField(columnInfo driverbase.ColumnInfo) arrow.Fi
442442

443443
switch driverbase.ValueOrZero(columnInfo.XdbcTypeName) {
444444
case "NUMBER":
445-
if c.useHighPrecision {
445+
scale := driverbase.ValueOrZero(columnInfo.XdbcDecimalDigits)
446+
// Use Int64 only when useHighPrecision=false AND scale=0
447+
if !c.useHighPrecision && scale == 0 {
448+
field.Type = arrow.PrimitiveTypes.Int64
449+
} else {
450+
// Use Decimal128 for all other cases to preserve precision
446451
field.Type = &arrow.Decimal128Type{
447452
Precision: int32(driverbase.ValueOrZero(columnInfo.XdbcColumnSize)),
448-
Scale: int32(driverbase.ValueOrZero(columnInfo.XdbcDecimalDigits)),
449-
}
450-
} else {
451-
if driverbase.ValueOrZero(columnInfo.XdbcDecimalDigits) == 0 {
452-
field.Type = arrow.PrimitiveTypes.Int64
453-
} else {
454-
field.Type = arrow.PrimitiveTypes.Float64
453+
Scale: int32(scale),
455454
}
456455
}
457456
case "FLOAT":

go/adbc/driver/snowflake/driver_test.go

Lines changed: 215 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1935,12 +1935,16 @@ func (suite *SnowflakeTests) TestUseHighPrecision() {
19351935

19361936
suite.EqualValues(2, n)
19371937
suite.Truef(arrow.TypeEqual(arrow.PrimitiveTypes.Int64, rdr.Schema().Field(0).Type), "expected int64, got %s", rdr.Schema().Field(0).Type)
1938-
suite.Truef(arrow.TypeEqual(arrow.PrimitiveTypes.Float64, rdr.Schema().Field(1).Type), "expected float64, got %s", rdr.Schema().Field(1).Type)
1938+
// NUMBER(15,2) should now return Decimal128 even with useHighPrecision=false
1939+
expectedType := &arrow.Decimal128Type{Precision: 15, Scale: 2}
1940+
suite.Truef(arrow.TypeEqual(expectedType, rdr.Schema().Field(1).Type), "expected decimal128(15,2), got %s", rdr.Schema().Field(1).Type)
19391941
suite.True(rdr.Next())
19401942
rec := rdr.Record()
19411943

1942-
suite.Equal(1234567.89, rec.Column(1).(*array.Float64).Value(0))
1943-
suite.Equal(9876543210.99, rec.Column(1).(*array.Float64).Value(1))
1944+
// Get values from Decimal128 array and verify precision is preserved
1945+
decimalArray := rec.Column(1).(*array.Decimal128)
1946+
suite.Equal(1234567.89, decimalArray.Value(0).ToFloat64(2))
1947+
suite.Equal(9876543210.99, decimalArray.Value(1).ToFloat64(2))
19441948
}
19451949

19461950
func (suite *SnowflakeTests) TestDecimalHighPrecision() {
@@ -1992,13 +1996,18 @@ func (suite *SnowflakeTests) TestNonIntDecimalLowPrecision() {
19921996
defer rdr.Release()
19931997

19941998
suite.EqualValues(1, n)
1995-
suite.Truef(arrow.TypeEqual(arrow.PrimitiveTypes.Float64, rdr.Schema().Field(0).Type), "expected float64, got %s", rdr.Schema().Field(0).Type)
1999+
// Now expects Decimal128 for non-zero scale even with useHighPrecision=false
2000+
expectedType := &arrow.Decimal128Type{Precision: int32(precision), Scale: int32(scale)}
2001+
suite.Truef(arrow.TypeEqual(expectedType, rdr.Schema().Field(0).Type), "expected decimal128(%d,%d), got %s", precision, scale, rdr.Schema().Field(0).Type)
19962002
suite.True(rdr.Next())
19972003
rec := rdr.Record()
19982004

1999-
value := rec.Column(0).(*array.Float64).Value(0)
2000-
difference := math.Abs(number - value)
2001-
suite.Truef(difference < 1e-13, "expected %f, got %f", number, value)
2005+
// Get value from Decimal128 array
2006+
decimalArray := rec.Column(0).(*array.Decimal128)
2007+
actualDecimal := decimalArray.Value(0)
2008+
actualValue := actualDecimal.ToFloat64(int32(scale))
2009+
difference := math.Abs(number - actualValue)
2010+
suite.Truef(difference < 1e-13, "expected %f, got %f", number, actualValue)
20022011

20032012
suite.False(rdr.Next())
20042013
}
@@ -2037,6 +2046,205 @@ func (suite *SnowflakeTests) TestIntDecimalLowPrecision() {
20372046
}
20382047
}
20392048

2049+
func (suite *SnowflakeTests) TestDecimalPrecisionPreserved() {
2050+
// Test that NUMBER(38,10) with use_high_precision=false
2051+
// returns exact decimal values, not floating point approximations
2052+
2053+
testCases := []struct {
2054+
name string
2055+
query string
2056+
expected string
2057+
}{
2058+
{
2059+
name: "Simple decimal addition",
2060+
query: "SELECT CAST(0.1 AS NUMBER(10,2)) + CAST(0.2 AS NUMBER(10,2)) AS RESULT",
2061+
expected: "0.30",
2062+
},
2063+
{
2064+
name: "Large precise decimal",
2065+
query: "SELECT CAST('123456789.123456789' AS NUMBER(18,9)) AS RESULT",
2066+
expected: "123456789.123456789",
2067+
},
2068+
{
2069+
name: "Financial calculation",
2070+
query: "SELECT CAST('99999999.99' AS NUMBER(10,2)) AS RESULT",
2071+
expected: "99999999.99",
2072+
},
2073+
}
2074+
2075+
for _, tc := range testCases {
2076+
suite.Run(tc.name, func() {
2077+
suite.Require().NoError(suite.stmt.SetOption(driver.OptionUseHighPrecision, adbc.OptionValueDisabled))
2078+
suite.Require().NoError(suite.stmt.SetSqlQuery(tc.query))
2079+
rdr, _, err := suite.stmt.ExecuteQuery(suite.ctx)
2080+
suite.Require().NoError(err)
2081+
defer rdr.Release()
2082+
2083+
suite.True(rdr.Next())
2084+
rec := rdr.Record()
2085+
2086+
// Verify we got a Decimal128 type
2087+
decimalType, ok := rec.Column(0).DataType().(*arrow.Decimal128Type)
2088+
suite.True(ok, "expected Decimal128 type")
2089+
2090+
// Get the decimal value and convert to string for exact comparison
2091+
decimalArray := rec.Column(0).(*array.Decimal128)
2092+
actualDecimal := decimalArray.Value(0)
2093+
actualStr := actualDecimal.ToString(decimalType.Scale)
2094+
2095+
suite.Equal(tc.expected, actualStr, "Precision was not preserved")
2096+
suite.False(rdr.Next())
2097+
})
2098+
}
2099+
}
2100+
2101+
// TestNumberScaleWithLowPrecision specifically tests that NUMBER columns with scale > 0
2102+
// return Decimal128 (not Float64) when use_high_precision=false.
2103+
//
2104+
// - OLD CODE: Schema declared as Float64 for NUMBER(p,s>0) but data came as Int64 (scaled integers)
2105+
// - NEW CODE: Schema and data both use Decimal128, preserving precision
2106+
func (suite *SnowflakeTests) TestNumberScaleWithLowPrecision() {
2107+
suite.Require().NoError(suite.Quirks.DropTable(suite.cnxn, "NUMBER_SCALE_TEST"))
2108+
2109+
// Create a table with various NUMBER types with different scales
2110+
suite.Require().NoError(suite.stmt.SetSqlQuery(`CREATE OR REPLACE TABLE NUMBER_SCALE_TEST (
2111+
int_col NUMBER(10,0),
2112+
decimal_2 NUMBER(10,2),
2113+
decimal_4 NUMBER(12,4),
2114+
decimal_9 NUMBER(18,9),
2115+
large_decimal NUMBER(38,10)
2116+
)`))
2117+
_, err := suite.stmt.ExecuteUpdate(suite.ctx)
2118+
suite.Require().NoError(err)
2119+
2120+
// Insert test data that would reveal precision loss if converted to Float64
2121+
suite.Require().NoError(suite.stmt.SetSqlQuery(`INSERT INTO NUMBER_SCALE_TEST VALUES
2122+
(12345, 123.45, 1234.5678, 123456789.123456789, 12345678901234567890.1234567890),
2123+
(67890, 678.90, 6789.0123, 987654321.987654321, 98765432109876543210.9876543210)`))
2124+
_, err = suite.stmt.ExecuteUpdate(suite.ctx)
2125+
suite.Require().NoError(err)
2126+
2127+
// Test with use_high_precision=false
2128+
suite.Require().NoError(suite.stmt.SetOption(driver.OptionUseHighPrecision, adbc.OptionValueDisabled))
2129+
suite.Require().NoError(suite.stmt.SetSqlQuery("SELECT * FROM NUMBER_SCALE_TEST"))
2130+
rdr, n, err := suite.stmt.ExecuteQuery(suite.ctx)
2131+
suite.Require().NoError(err)
2132+
defer rdr.Release()
2133+
2134+
suite.EqualValues(2, n)
2135+
2136+
// Verify schema types
2137+
schema := rdr.Schema()
2138+
2139+
// int_col: NUMBER(10,0) should be Int64
2140+
suite.Truef(arrow.TypeEqual(arrow.PrimitiveTypes.Int64, schema.Field(0).Type),
2141+
"NUMBER(10,0) expected int64, got %s", schema.Field(0).Type)
2142+
2143+
// decimal_2: NUMBER(10,2) should be Decimal128 (NOT Float64)
2144+
suite.Truef(arrow.TypeEqual(&arrow.Decimal128Type{Precision: 10, Scale: 2}, schema.Field(1).Type),
2145+
"NUMBER(10,2) expected decimal128(10,2), got %s - if Float64, the bug is present!", schema.Field(1).Type)
2146+
2147+
// decimal_4: NUMBER(12,4) should be Decimal128
2148+
suite.Truef(arrow.TypeEqual(&arrow.Decimal128Type{Precision: 12, Scale: 4}, schema.Field(2).Type),
2149+
"NUMBER(12,4) expected decimal128(12,4), got %s", schema.Field(2).Type)
2150+
2151+
// decimal_9: NUMBER(18,9) should be Decimal128
2152+
suite.Truef(arrow.TypeEqual(&arrow.Decimal128Type{Precision: 18, Scale: 9}, schema.Field(3).Type),
2153+
"NUMBER(18,9) expected decimal128(18,9), got %s", schema.Field(3).Type)
2154+
2155+
// large_decimal: NUMBER(38,10) should be Decimal128
2156+
suite.Truef(arrow.TypeEqual(&arrow.Decimal128Type{Precision: 38, Scale: 10}, schema.Field(4).Type),
2157+
"NUMBER(38,10) expected decimal128(38,10), got %s", schema.Field(4).Type)
2158+
2159+
// Verify data values are preserved correctly
2160+
suite.True(rdr.Next())
2161+
rec := rdr.Record()
2162+
2163+
// Check first row values
2164+
suite.Equal(int64(12345), rec.Column(0).(*array.Int64).Value(0))
2165+
2166+
// For decimal columns, verify exact values are preserved
2167+
dec2, ok := rec.Column(1).(*array.Decimal128)
2168+
suite.True(ok, "Failed to cast column 1 to Decimal128 - this means data type doesn't match schema!")
2169+
suite.Equal("123.45", dec2.Value(0).ToString(2))
2170+
2171+
dec4 := rec.Column(2).(*array.Decimal128)
2172+
suite.Equal("1234.5678", dec4.Value(0).ToString(4))
2173+
2174+
dec9 := rec.Column(3).(*array.Decimal128)
2175+
suite.Equal("123456789.123456789", dec9.Value(0).ToString(9))
2176+
2177+
dec38 := rec.Column(4).(*array.Decimal128)
2178+
suite.Equal("12345678901234567890.1234567890", dec38.Value(0).ToString(10))
2179+
2180+
// Check second row to ensure consistency
2181+
suite.True(rdr.Next())
2182+
rec = rdr.Record()
2183+
2184+
suite.Equal(int64(67890), rec.Column(0).(*array.Int64).Value(0))
2185+
suite.Equal("678.90", rec.Column(1).(*array.Decimal128).Value(0).ToString(2))
2186+
suite.Equal("6789.0123", rec.Column(2).(*array.Decimal128).Value(0).ToString(4))
2187+
suite.Equal("987654321.987654321", rec.Column(3).(*array.Decimal128).Value(0).ToString(9))
2188+
suite.Equal("98765432109876543210.9876543210", rec.Column(4).(*array.Decimal128).Value(0).ToString(10))
2189+
2190+
suite.False(rdr.Next())
2191+
}
2192+
2193+
// TestNumberInt64DataPath verifies that when Snowflake returns NUMBER data as scaled Int64
2194+
// (the default for performance), the conversion to Decimal128 works correctly.
2195+
// This specifically tests the code path in getTransformer that handles Int64 -> Decimal128.
2196+
func (suite *SnowflakeTests) TestNumberInt64DataPath() {
2197+
// Force Snowflake to return data as Int64 by using specific precision/scale combinations
2198+
testCases := []struct {
2199+
precision int
2200+
scale int
2201+
value string
2202+
expected string
2203+
}{
2204+
// Test various precision/scale combinations that trigger Int64 data path
2205+
{10, 2, "123.45", "123.45"},
2206+
{15, 2, "1234567890.12", "1234567890.12"},
2207+
{18, 4, "12345678901234.5678", "12345678901234.5678"},
2208+
{38, 10, "1234567890123456789012345678.1234567890", "1234567890123456789012345678.1234567890"},
2209+
// Test negative values
2210+
{10, 2, "-999.99", "-999.99"},
2211+
{15, 3, "-123456789.123", "-123456789.123"},
2212+
// Test edge cases
2213+
{10, 2, "0.01", "0.01"},
2214+
{10, 2, "-0.01", "-0.01"},
2215+
}
2216+
2217+
for _, tc := range testCases {
2218+
suite.Run(fmt.Sprintf("NUMBER(%d,%d)=%s", tc.precision, tc.scale, tc.value), func() {
2219+
query := fmt.Sprintf("SELECT CAST('%s' AS NUMBER(%d,%d)) AS result", tc.value, tc.precision, tc.scale)
2220+
2221+
// Test with use_high_precision=false to ensure Decimal128 is used
2222+
suite.Require().NoError(suite.stmt.SetOption(driver.OptionUseHighPrecision, adbc.OptionValueDisabled))
2223+
suite.Require().NoError(suite.stmt.SetSqlQuery(query))
2224+
2225+
rdr, _, err := suite.stmt.ExecuteQuery(suite.ctx)
2226+
suite.Require().NoError(err)
2227+
defer rdr.Release()
2228+
2229+
// Verify schema type is Decimal128
2230+
expectedType := &arrow.Decimal128Type{Precision: int32(tc.precision), Scale: int32(tc.scale)}
2231+
actualType := rdr.Schema().Field(0).Type
2232+
suite.Truef(arrow.TypeEqual(expectedType, actualType),
2233+
"Expected decimal128(%d,%d), got %s", tc.precision, tc.scale, actualType)
2234+
2235+
// Verify the value is preserved exactly
2236+
suite.True(rdr.Next())
2237+
rec := rdr.Record()
2238+
2239+
decArray := rec.Column(0).(*array.Decimal128)
2240+
actualValue := decArray.Value(0).ToString(int32(tc.scale))
2241+
suite.Equal(tc.expected, actualValue, "Value not preserved exactly")
2242+
2243+
suite.False(rdr.Next())
2244+
})
2245+
}
2246+
}
2247+
20402248
func (suite *SnowflakeTests) TestDescribeOnly() {
20412249
suite.Require().NoError(suite.stmt.SetOption(driver.OptionUseHighPrecision, adbc.OptionValueEnabled))
20422250
suite.Require().NoError(suite.stmt.SetSqlQuery("SELECT CAST('9999.99' AS NUMBER(6, 2)) AS RESULT"))

0 commit comments

Comments
 (0)