From 30e08eb53228e6e5dca6511a394eac3210ac4f13 Mon Sep 17 00:00:00 2001 From: Travis Date: Sun, 15 Mar 2020 16:18:59 -0500 Subject: [PATCH] add error conditions to tests --- executor_test.go | 8 ++--- pql/decimal.go | 19 ++++++++++-- pql/decimal_test.go | 75 ++++++++++++++++++++++++++++++++------------- 3 files changed, 73 insertions(+), 29 deletions(-) diff --git a/executor_test.go b/executor_test.go index b695fcbe8..777406dc3 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1481,10 +1481,10 @@ func TestExecutor_Execute_MinMax(t *testing.T) { max int64 set pql.Decimal }{ - {2, 10, 20, pql.Decimal{false, 115, 1}}, - {2, -10, 20, pql.Decimal{false, 115, 1}}, - {2, -10, 20, pql.Decimal{true, 95, 1}}, - {2, -20, -10, pql.Decimal{true, 115, 1}}, + {2, 10, 20, pql.Decimal{Sign: false, Value: 115, Scale: 1}}, + {2, -10, 20, pql.Decimal{Sign: false, Value: 115, Scale: 1}}, + {2, -10, 20, pql.Decimal{Sign: true, Value: 95, Scale: 1}}, + {2, -20, -10, pql.Decimal{Sign: true, Value: 115, Scale: 1}}, } for i, test := range tests { fld := fmt.Sprintf("f%d", i) diff --git a/pql/decimal.go b/pql/decimal.go index b0c6e58c3..af56b766c 100644 --- a/pql/decimal.go +++ b/pql/decimal.go @@ -117,9 +117,6 @@ const ( // ParseDecimal parses a string into a Decimal. func ParseDecimal(s string) (Decimal, error) { - if s == "" { - return Decimal{}, nil - } var sign bool var value uint64 var scale int64 @@ -140,6 +137,7 @@ func ParseDecimal(s string) (Decimal, error) { mantissa := make([]byte, len(s)) state := stateSign + var foundLeadingZero bool for i := 0; i < len(s); i++ { switch state { case stateSign: @@ -158,6 +156,7 @@ func ParseDecimal(s string) (Decimal, error) { case stateLeadingZeros: switch s[i] { case '0': + foundLeadingZero = true continue default: state = stateMantissa @@ -179,6 +178,13 @@ func ParseDecimal(s string) (Decimal, error) { } } + // If we've gotten here and state is still in stateSign or + // it's in stateLeadingZeros without finding any zeros, + // it means no value was provided. + if state == stateSign || (state == stateLeadingZeros && !foundLeadingZero) { + return Decimal{}, errors.New("decimal string is empty") + } + // Trim trailing zeros/spaces of mantissa // for any portion that would have been to the // right of the decimal. @@ -205,6 +211,13 @@ func ParseDecimal(s string) (Decimal, error) { scale = int64(len(mantissa) - decimalPos) } + // If mantissa is empty, treat it as "0". + if len(mantissa) == 0 { + mantissa = []byte{'0'} + sign = false + scale = 0 + } + value, err = strconv.ParseUint(string(mantissa), 10, 32) if err != nil { return Decimal{}, errors.Wrap(err, "converting mantissa to uint32") diff --git a/pql/decimal_test.go b/pql/decimal_test.go index 90651d731..4368767fe 100644 --- a/pql/decimal_test.go +++ b/pql/decimal_test.go @@ -15,6 +15,7 @@ package pql_test import ( + "strings" "testing" "github.com/pilosa/pilosa/v2/pql" @@ -26,34 +27,64 @@ func TestDecimal(t *testing.T) { tests := []struct { s string exp pql.Decimal - expErr error // TODO: add tests for errors + expErr string }{ - {"123.4567", pql.Decimal{false, 1234567, 4}, nil}, - {" 123.4567", pql.Decimal{false, 1234567, 4}, nil}, - {" 123.4567 ", pql.Decimal{false, 1234567, 4}, nil}, - {"123.456700", pql.Decimal{false, 1234567, 4}, nil}, - {"00123.4567", pql.Decimal{false, 1234567, 4}, nil}, - {"+123.4567", pql.Decimal{false, 1234567, 4}, nil}, - {"-123.4567", pql.Decimal{true, 1234567, 4}, nil}, - {"-00123.4567", pql.Decimal{true, 1234567, 4}, nil}, - {"-12.25", pql.Decimal{true, 1225, 2}, nil}, + {"0", pql.Decimal{false, 0, 0}, ""}, + {"-0", pql.Decimal{false, 0, 0}, ""}, + {"0.0", pql.Decimal{false, 0, 0}, ""}, + {"-0.00", pql.Decimal{false, 0, 0}, ""}, + {"123.4567", pql.Decimal{false, 1234567, 4}, ""}, + {" 123.4567", pql.Decimal{false, 1234567, 4}, ""}, + {" 123.4567 ", pql.Decimal{false, 1234567, 4}, ""}, + {"123.456700", pql.Decimal{false, 1234567, 4}, ""}, + {"00123.4567", pql.Decimal{false, 1234567, 4}, ""}, + {"+123.4567", pql.Decimal{false, 1234567, 4}, ""}, + {"-123.4567", pql.Decimal{true, 1234567, 4}, ""}, + {"-00123.4567", pql.Decimal{true, 1234567, 4}, ""}, + {"-12.25", pql.Decimal{true, 1225, 2}, ""}, - {"123", pql.Decimal{false, 123, 0}, nil}, - {"-12300", pql.Decimal{true, 123, -2}, nil}, - {"+012300", pql.Decimal{false, 123, -2}, nil}, - {"12300", pql.Decimal{false, 123, -2}, nil}, - {"12300.", pql.Decimal{false, 123, -2}, nil}, - {"12300.0", pql.Decimal{false, 123, -2}, nil}, - {"123.0", pql.Decimal{false, 123, 0}, nil}, + {"123", pql.Decimal{false, 123, 0}, ""}, + {"-12300", pql.Decimal{true, 123, -2}, ""}, + {"+012300", pql.Decimal{false, 123, -2}, ""}, + {"12300", pql.Decimal{false, 123, -2}, ""}, + {"12300.", pql.Decimal{false, 123, -2}, ""}, + {"12300.0", pql.Decimal{false, 123, -2}, ""}, + {"123.0", pql.Decimal{false, 123, 0}, ""}, - {"0.123", pql.Decimal{false, 123, 3}, nil}, - {"0.001230", pql.Decimal{false, 123, 5}, nil}, - {" 0.001230 ", pql.Decimal{false, 123, 5}, nil}, - {"-0.001230 ", pql.Decimal{true, 123, 5}, nil}, + {".123", pql.Decimal{false, 123, 3}, ""}, + {"0.123", pql.Decimal{false, 123, 3}, ""}, + {"0.001230", pql.Decimal{false, 123, 5}, ""}, + {" 0.001230 ", pql.Decimal{false, 123, 5}, ""}, + {"-0.001230 ", pql.Decimal{true, 123, 5}, ""}, + + // uint32 edges. + {".000004294967295", pql.Decimal{false, 4294967295, 15}, ""}, + {"-.000004294967295", pql.Decimal{true, 4294967295, 15}, ""}, + {"-42949.67295", pql.Decimal{true, 4294967295, 5}, ""}, + {"42949.67295", pql.Decimal{false, 4294967295, 5}, ""}, + {"-42949.67295", pql.Decimal{true, 4294967295, 5}, ""}, + {"4294967295000", pql.Decimal{false, 4294967295, -3}, ""}, + {"-4294967295000", pql.Decimal{true, 4294967295, -3}, ""}, + + // Error cases. + {"", pql.Decimal{}, "decimal string is empty"}, + {"-", pql.Decimal{}, "decimal string is empty"}, + {"*0.123", pql.Decimal{}, "invalid syntax"}, + {"abc", pql.Decimal{}, "invalid syntax"}, + {"0.12.3", pql.Decimal{}, "invalid decimal string"}, + {"--12300", pql.Decimal{}, "invalid syntax"}, + {"429496729.6", pql.Decimal{}, "value out of range"}, + {"-429496729.6", pql.Decimal{}, "value out of range"}, + {"4294967296000", pql.Decimal{}, "value out of range"}, + {"-4294967296000", pql.Decimal{}, "value out of range"}, } for i, test := range tests { dec, err := pql.ParseDecimal(test.s) - if err != nil { + if test.expErr != "" { + if err == nil || !strings.Contains(err.Error(), test.expErr) { + t.Fatalf("expected error to contain: %s, but got: %v", test.expErr, err) + } + } else if err != nil { t.Fatalf("parsing string `%s`: %s", test.s, err) } else if dec != test.exp { t.Fatalf("test %d expected: %v, but got: %v", i, test.exp, dec)