Skip to content

Commit b971a87

Browse files
authored
fix(clickhouse): exact microsecond->Time64 conversion on snapshot path (#4378)
1 parent c6a6e54 commit b971a87

3 files changed

Lines changed: 14 additions & 4 deletions

File tree

flow/connectors/clickhouse/table_function.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ func timeFieldExpressionConverter(
7676
}
7777

7878
// QValueTime is stored as time-micro logical type in Avro, toTime64 accepts a
79-
// fractional second, so conversion is necessary
80-
return fmt.Sprintf("toTime64(%s / 1000000.0, 6)", sourceFieldIdentifier), nil
79+
// fractional second, so conversion is necessary. Trip through Decimal64 to preserve precision.
80+
return fmt.Sprintf("toTime64(toDecimal64(%s, 6) / 1000000, 6)", sourceFieldIdentifier), nil
8181
}
8282

8383
var defaultFieldExpressionConverters = []fieldExpressionConverter{

flow/e2e/clickhouse.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,15 +175,15 @@ func (s ClickHouseSuite) GetRows(table string, cols string) (*model.QRecordBatch
175175

176176
batch := &model.QRecordBatch{}
177177
colTypes := rows.ColumnTypes()
178-
row := make([]any, 0, len(colTypes))
178+
scanTypes := make([]reflect.Type, len(colTypes))
179179
tableSchema, err := connclickhouse.GetTableSchemaForTable(&protos.TableMapping{SourceTableIdentifier: table}, colTypes)
180180
if err != nil {
181181
return nil, err
182182
}
183183

184184
for idx, ty := range colTypes {
185185
fieldDesc := tableSchema.Columns[idx]
186-
row = append(row, reflect.New(ty.ScanType()).Interface())
186+
scanTypes[idx] = ty.ScanType()
187187
batch.Schema.Fields = append(batch.Schema.Fields, types.QField{
188188
Name: ty.Name(),
189189
Type: types.QValueKind(fieldDesc.Type),
@@ -194,6 +194,13 @@ func (s ClickHouseSuite) GetRows(table string, cols string) (*model.QRecordBatch
194194
}
195195

196196
for rows.Next() {
197+
// Allocate fresh scan targets per row: clickhouse-go does not reset a
198+
// nullable destination to nil when scanning a NULL, so a reused buffer
199+
// would make a NULL row inherit the previous row's value.
200+
row := make([]any, len(scanTypes))
201+
for idx, st := range scanTypes {
202+
row[idx] = reflect.New(st).Interface()
203+
}
197204
if err := rows.Scan(row...); err != nil {
198205
return nil, err
199206
}

flow/e2e/clickhouse_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,6 +1348,9 @@ func (s ClickHouseSuite) Test_Time64() {
13481348
`, srcFullName)))
13491349
require.NoError(s.t, s.source.Exec(s.t.Context(), fmt.Sprintf(
13501350
`INSERT INTO %s (t, t_nullable, t_nullable_2) VALUES ('14:21:00', '08:30:00.123456', NULL)`, srcFullName)))
1351+
// Regression test for precise conversion - these values were losing 1us when tripped through floating point
1352+
require.NoError(s.t, s.source.Exec(s.t.Context(), fmt.Sprintf(
1353+
`INSERT INTO %s (t, t_nullable, t_nullable_2) VALUES ('00:00:00.000249', '00:00:00.000251', '00:00:00.000493')`, srcFullName)))
13511354

13521355
connectionGen := FlowConnectionGenerationConfig{
13531356
FlowJobName: s.attachSuffix(srcTableName),

0 commit comments

Comments
 (0)