From 298f290e86f177b52f3d4839e6f3457275658800 Mon Sep 17 00:00:00 2001 From: Travis Date: Tue, 17 Mar 2020 23:08:35 -0500 Subject: [PATCH] Avoid overflow on decimal min/max default values If the min/max provided are already on the boundary of int64, then we don't want to operate on them and cause overflow. there are still overflow scenarios where a user provides a min/max which is not on the boundary, but overflow once the scale is applied. This does not address those cases, but at least it addresses the default case (where a min/max is not provided) --- field.go | 17 ++++++++++++++--- field_internal_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/field.go b/field.go index aabcf0f0a..046159c76 100644 --- a/field.go +++ b/field.go @@ -195,8 +195,19 @@ func OptFieldTypeDecimal(scale int64, minmax ...int64) FieldOption { if len(minmax) == 2 { min, max := minmax[0], minmax[1] if scale != 0 { - min = int64(float64(min) * math.Pow10(int(scale))) - max = int64(float64(max) * math.Pow10(int(scale))) + // If the min/max provided are already on the boundary of int64, + // then we don't want to operate on them and cause overflow. + // There are still overflow scenarios where a user provides a + // min/max which is not on the boundary, but overflow once the + // scale is applied. This does not address those cases, but at + // least it addresses the default case (where a min/max is not + // provided). + if min != math.MinInt64 { + min = int64(float64(min) * math.Pow10(int(scale))) + } + if max != math.MaxInt64 { + max = int64(float64(max) * math.Pow10(int(scale))) + } } if min > max { return errors.Errorf("decimal field min cannot be greater than max, got %d, %d", min, max) @@ -208,7 +219,7 @@ func OptFieldTypeDecimal(scale int64, minmax ...int64) FieldOption { } else if len(minmax) == 1 { // It's not necessary to handle the scale==0 case separately, // but it avoids the type conversion. - if scale == 0 { + if scale == 0 || minmax[0] == math.MinInt64 { fo.Min = minmax[0] } else { fo.Min = int64(float64(minmax[0]) * math.Pow10(int(scale))) diff --git a/field_internal_test.go b/field_internal_test.go index 38a023c1f..c7a3015a1 100644 --- a/field_internal_test.go +++ b/field_internal_test.go @@ -654,6 +654,31 @@ func TestIntField_MinMaxForShard(t *testing.T) { } } +func TestDecimalField_MinMaxBoundaries(t *testing.T) { + for i, test := range []struct { + min int64 + max int64 + scale int64 + expmin int64 + expmax int64 + }{ + {min: math.MinInt64, max: math.MaxInt64, scale: 3, expmin: math.MinInt64, expmax: math.MaxInt64}, + {min: 44, max: 88, scale: 3, expmin: 44000, expmax: 88000}, + {min: -44, max: 88, scale: 3, expmin: -44000, expmax: 88000}, + } { + t.Run("minmax"+strconv.Itoa(i), func(t *testing.T) { + f := MustOpenField(OptFieldTypeDecimal(test.scale, test.min, test.max)) + + if f.Options().Min != test.expmin { + t.Fatalf("expected min: %v, but got: %v", test.expmin, f.Options().Min) + } + if f.Options().Max != test.expmax { + t.Fatalf("expected max: %v, but got: %v", test.expmax, f.Options().Max) + } + }) + } +} + func TestDecimalField_MinMaxForShard(t *testing.T) { f := MustOpenField(OptFieldTypeDecimal(3))