Skip to content

Commit 0ef315f

Browse files
flarcoflarco
committed
fix: enhance decimal precision and scale handling in ColumnTyping and SQLColumns, add tests for ColumnTyping
1 parent 8b7c91a commit 0ef315f

File tree

4 files changed

+223
-4
lines changed

4 files changed

+223
-4
lines changed

‎core/dbio/database/database.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,9 +1495,6 @@ func SQLColumns(colTypes []ColumnType, conn Connection) (columns iop.Columns) {
14951495
}
14961496
} else {
14971497
col.Sourced = false
1498-
precisionDelta := lo.Ternary(col.DbPrecision > env.DdlMinDecLength, col.DbPrecision-env.DdlMinDecLength, 0)
1499-
scaleDelta := lo.Ternary(col.DbScale > env.DdlMinDecScale, col.DbScale-env.DdlMinDecScale, 0)
1500-
col.DbPrecision = env.DdlMinDecLength + precisionDelta + scaleDelta // safe if scale if present
15011498
}
15021499
}
15031500

‎core/dbio/iop/datatype.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,6 +1570,10 @@ func (dct *DecimalColumnTyping) Apply(col *Column) (precision, scale int) {
15701570
dct.MaxScale = lo.Ternary(dct.MaxScale == 0, env.DdlMaxDecScale, dct.MaxScale)
15711571
dct.MinPrecision = lo.Ternary(dct.MinPrecision == nil, g.Ptr(env.DdlMinDecLength), dct.MinPrecision)
15721572
dct.MaxPrecision = lo.Ternary(dct.MaxPrecision == 0, env.DdlMaxDecLength, dct.MaxPrecision)
1573+
1574+
precisionDelta := lo.Ternary(precision > env.DdlMinDecLength, precision-env.DdlMinDecLength, 0)
1575+
scaleDelta := lo.Ternary(scale > env.DdlMinDecScale, scale-env.DdlMinDecScale, 0)
1576+
precision = env.DdlMinDecLength + precisionDelta + scaleDelta // safe if scale if present
15731577
}
15741578

15751579
if dct.MinPrecision != nil && precision < *dct.MinPrecision {

‎core/dbio/iop/datatype_test.go‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,224 @@ func TestParseDecimal(t *testing.T) {
324324
assert.Error(t, err)
325325
}
326326

327+
func TestColumnTyping(t *testing.T) {
328+
maxStringLength := 1000
329+
330+
type testCase struct {
331+
name string
332+
column Column
333+
columnTyping ColumnTyping
334+
335+
expectedDecimalPrecision int
336+
expectedDecimalScale int
337+
expectedStringLength int
338+
}
339+
340+
testCases := []testCase{
341+
// Decimal column typing tests
342+
{
343+
name: "decimal_sourced_precision_scale",
344+
column: Column{Name: "test", Type: DecimalType, DbPrecision: 10, DbScale: 2, Sourced: true},
345+
columnTyping: ColumnTyping{Decimal: &DecimalColumnTyping{}},
346+
expectedDecimalPrecision: 10,
347+
expectedDecimalScale: 2,
348+
},
349+
{
350+
name: "decimal_sourced_precision_scale_2",
351+
column: Column{Name: "test", Type: DecimalType, DbPrecision: 10, DbScale: 2, Sourced: true},
352+
columnTyping: ColumnTyping{Decimal: &DecimalColumnTyping{}},
353+
expectedDecimalPrecision: 10,
354+
expectedDecimalScale: 2,
355+
},
356+
{
357+
name: "decimal_min_precision_scale",
358+
column: Column{Name: "test", Type: DecimalType, DbPrecision: 5, DbScale: 1, Sourced: false},
359+
columnTyping: ColumnTyping{Decimal: &DecimalColumnTyping{MinPrecision: g.Ptr(10), MinScale: g.Ptr(3)}},
360+
expectedDecimalPrecision: 24,
361+
expectedDecimalScale: 3,
362+
},
363+
{
364+
name: "decimal_max_precision_scale",
365+
column: Column{Name: "test", Type: DecimalType, DbPrecision: 50, DbScale: 15, Sourced: false},
366+
columnTyping: ColumnTyping{Decimal: &DecimalColumnTyping{MaxPrecision: 20, MaxScale: 10}},
367+
expectedDecimalPrecision: 20,
368+
expectedDecimalScale: 10,
369+
},
370+
{
371+
name: "decimal_with_stats",
372+
column: Column{Name: "test", Type: DecimalType, Stats: ColumnStats{MaxLen: 8, MaxDecLen: 3}, Sourced: false},
373+
columnTyping: ColumnTyping{Decimal: &DecimalColumnTyping{}},
374+
expectedDecimalPrecision: 24,
375+
expectedDecimalScale: 6,
376+
},
377+
{
378+
name: "decimal_zero_precision_scale",
379+
column: Column{Name: "test", Type: DecimalType, DbPrecision: 0, DbScale: 0, Sourced: false},
380+
columnTyping: ColumnTyping{Decimal: &DecimalColumnTyping{}},
381+
expectedDecimalPrecision: 24,
382+
expectedDecimalScale: 6,
383+
},
384+
{
385+
name: "decimal_delta",
386+
column: Column{Name: "test", Type: DecimalType, DbPrecision: 0, DbScale: 19, Sourced: false},
387+
columnTyping: ColumnTyping{Decimal: &DecimalColumnTyping{}},
388+
expectedDecimalPrecision: 38,
389+
expectedDecimalScale: 19,
390+
},
391+
392+
// String column typing tests
393+
{
394+
name: "string_basic_length",
395+
column: Column{Name: "test", Type: StringType, Stats: ColumnStats{MaxLen: 50}},
396+
columnTyping: ColumnTyping{String: &StringColumnTyping{}},
397+
expectedStringLength: 50,
398+
},
399+
{
400+
name: "string_length_factor",
401+
column: Column{Name: "test", Type: StringType, Stats: ColumnStats{MaxLen: 50}},
402+
columnTyping: ColumnTyping{String: &StringColumnTyping{LengthFactor: 2}},
403+
expectedStringLength: 100,
404+
},
405+
{
406+
name: "string_length_factor_exceeds_max",
407+
column: Column{Name: "test", Type: StringType, Stats: ColumnStats{MaxLen: 600}},
408+
columnTyping: ColumnTyping{String: &StringColumnTyping{LengthFactor: 2}},
409+
expectedStringLength: 1000, // should cap at maxStringLength
410+
},
411+
{
412+
name: "string_min_length",
413+
column: Column{Name: "test", Type: StringType, Stats: ColumnStats{MaxLen: 10}},
414+
columnTyping: ColumnTyping{String: &StringColumnTyping{MinLength: 50}},
415+
expectedStringLength: 50,
416+
},
417+
{
418+
name: "string_max_length",
419+
column: Column{Name: "test", Type: StringType, Stats: ColumnStats{MaxLen: 200}},
420+
columnTyping: ColumnTyping{String: &StringColumnTyping{MaxLength: 150}},
421+
expectedStringLength: 200, // original length since MaxLength doesn't override max
422+
},
423+
{
424+
name: "string_use_max",
425+
column: Column{Name: "test", Type: StringType, Stats: ColumnStats{MaxLen: 50}},
426+
columnTyping: ColumnTyping{String: &StringColumnTyping{UseMax: true}},
427+
expectedStringLength: 1000, // should use maxStringLength
428+
},
429+
{
430+
name: "string_use_max_with_custom_max",
431+
column: Column{Name: "test", Type: StringType, Stats: ColumnStats{MaxLen: 50}},
432+
columnTyping: ColumnTyping{String: &StringColumnTyping{UseMax: true, MaxLength: 2000}},
433+
expectedStringLength: 2000, // should use custom MaxLength
434+
},
435+
{
436+
name: "string_min_length_with_factor",
437+
column: Column{Name: "test", Type: StringType, Stats: ColumnStats{MaxLen: 10}},
438+
columnTyping: ColumnTyping{String: &StringColumnTyping{LengthFactor: 2, MinLength: 50}},
439+
expectedStringLength: 50, // factor gives 20, but min is 50
440+
},
441+
442+
// Sourced column precision tests
443+
{
444+
name: "string_sourced_precision",
445+
column: Column{Name: "test", Type: StringType, DbPrecision: 100, Sourced: true},
446+
columnTyping: ColumnTyping{String: &StringColumnTyping{}},
447+
expectedStringLength: 100,
448+
},
449+
{
450+
name: "string_sourced_precision_with_factor",
451+
column: Column{Name: "test", Type: StringType, DbPrecision: 50, Sourced: true},
452+
columnTyping: ColumnTyping{String: &StringColumnTyping{LengthFactor: 2}},
453+
expectedStringLength: 100,
454+
},
455+
}
456+
457+
for _, testCase := range testCases {
458+
t.Run(testCase.name, func(t *testing.T) {
459+
if sct := testCase.columnTyping.String; sct != nil {
460+
var length int
461+
if testCase.column.Sourced && testCase.column.DbPrecision > 0 {
462+
length = sct.Apply(testCase.column.DbPrecision, maxStringLength)
463+
} else {
464+
length = sct.Apply(testCase.column.Stats.MaxLen, maxStringLength)
465+
}
466+
assert.Equal(t, testCase.expectedStringLength, length)
467+
468+
} else if dct := testCase.columnTyping.Decimal; dct != nil {
469+
precision, scale := dct.Apply(&testCase.column)
470+
assert.Equal(t, testCase.expectedDecimalPrecision, precision)
471+
assert.Equal(t, testCase.expectedDecimalScale, scale)
472+
}
473+
})
474+
}
475+
476+
// Keep the original hardcoded test for backward compatibility
477+
col := Column{Name: "test", Type: DecimalType, DbPrecision: 10, DbScale: 0, Sourced: true}
478+
ct := ColumnTyping{Decimal: &DecimalColumnTyping{}}
479+
precision, scale := ct.Decimal.Apply(&col)
480+
assert.Equal(t, 10, precision)
481+
assert.Equal(t, 0, scale)
482+
}
483+
484+
// Additional test for JSON column typing
485+
func TestColumnTypingJSON(t *testing.T) {
486+
t.Run("json_as_text_false", func(t *testing.T) {
487+
col := Column{Name: "test", Type: JsonType}
488+
jct := JsonColumnTyping{AsText: false}
489+
jct.Apply(&col)
490+
assert.Equal(t, JsonType, col.Type)
491+
})
492+
493+
t.Run("json_as_text_true", func(t *testing.T) {
494+
col := Column{Name: "test", Type: JsonType}
495+
jct := JsonColumnTyping{AsText: true}
496+
jct.Apply(&col)
497+
assert.Equal(t, TextType, col.Type)
498+
})
499+
}
500+
501+
// Test for MaxDecimals method
502+
func TestColumnTypingMaxDecimals(t *testing.T) {
503+
t.Run("nil_column_typing", func(t *testing.T) {
504+
var ct *ColumnTyping
505+
assert.Equal(t, -1, ct.MaxDecimals())
506+
})
507+
508+
t.Run("nil_decimal_typing", func(t *testing.T) {
509+
ct := &ColumnTyping{}
510+
assert.Equal(t, -1, ct.MaxDecimals())
511+
})
512+
513+
t.Run("max_scale_set", func(t *testing.T) {
514+
ct := &ColumnTyping{
515+
Decimal: &DecimalColumnTyping{MaxScale: 5},
516+
}
517+
assert.Equal(t, 5, ct.MaxDecimals())
518+
})
519+
520+
t.Run("min_scale_set_no_max", func(t *testing.T) {
521+
ct := &ColumnTyping{
522+
Decimal: &DecimalColumnTyping{MinScale: g.Ptr(3)},
523+
}
524+
assert.Equal(t, 3, ct.MaxDecimals())
525+
})
526+
527+
t.Run("both_scales_set", func(t *testing.T) {
528+
ct := &ColumnTyping{
529+
Decimal: &DecimalColumnTyping{
530+
MaxScale: 5,
531+
MinScale: g.Ptr(3),
532+
},
533+
}
534+
assert.Equal(t, 5, ct.MaxDecimals()) // MaxScale takes precedence
535+
})
536+
537+
t.Run("no_scales_set", func(t *testing.T) {
538+
ct := &ColumnTyping{
539+
Decimal: &DecimalColumnTyping{},
540+
}
541+
assert.Equal(t, -1, ct.MaxDecimals())
542+
})
543+
}
544+
327545
func TestDatasetSort(t *testing.T) {
328546
columns := NewColumnsFromFields("col1", "col2")
329547
data := NewDataset(columns)

‎core/dbio/scripts/test.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ go test -v -run 'TestConnection'
88
cd -
99

1010
cd iop
11-
go test -timeout 5m -v -run 'TestParseDate|TestDetectDelimiter|TestFIX|TestConstraints|TestDuckDb|TestParquetDuckDb|TestIcebergReader|TestDeltaReader|TestPartition|TestExtractPartitionTimeValue|TestGetLowestPartTimeUnit|TestMatchedPartitionMask|TestGeneratePartURIsFromRange|TestDataset|TestValidateNames|TestExcelDateToTime|TestBinaryToHex|TestBinaryToDecimal|TestArrow|TestFunctions|TestQueue|TestEvaluator|TestTransforms'
11+
go test -timeout 5m -v -run 'TestParseDate|TestDetectDelimiter|TestFIX|TestConstraints|TestDuckDb|TestParquetDuckDb|TestIcebergReader|TestDeltaReader|TestPartition|TestExtractPartitionTimeValue|TestGetLowestPartTimeUnit|TestMatchedPartitionMask|TestGeneratePartURIsFromRange|TestDataset|TestValidateNames|TestExcelDateToTime|TestBinaryToHex|TestBinaryToDecimal|TestArrow|TestFunctions|TestQueue|TestEvaluator|TestTransforms|TestColumnTyping'
1212
cd -
1313

1414
cd database

0 commit comments

Comments
 (0)