From c60241b5a9855bb79c2ac6dc01120f403a1546a3 Mon Sep 17 00:00:00 2001 From: Travis Date: Sun, 15 Mar 2020 23:00:00 -0500 Subject: [PATCH] Get rid of Sign from pql.Decimal struct It turns out that it's not very useful to keep the sign value as a separate argument in the pql.Decimal struct. This commit incorporates it into Value, and makes Value an `int64` (for some bone-headed reason I had made it a `uint32` before which is just dumb). --- executor_test.go | 8 ++-- pql/decimal.go | 41 ++++++++++------- pql/decimal_test.go | 105 ++++++++++++++++++++++---------------------- pql/parser_test.go | 13 +++--- 4 files changed, 87 insertions(+), 80 deletions(-) diff --git a/executor_test.go b/executor_test.go index 777406dc3..67e508be0 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{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}}, + {2, 10, 20, pql.Decimal{Value: 115, Scale: 1}}, + {2, -10, 20, pql.Decimal{Value: 115, Scale: 1}}, + {2, -10, 20, pql.Decimal{Value: -95, Scale: 1}}, + {2, -20, -10, pql.Decimal{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 af56b766c..f93e4826b 100644 --- a/pql/decimal.go +++ b/pql/decimal.go @@ -24,17 +24,15 @@ import ( ) // Decimal represents a decimal value; the intention -// is to avoid relying on float64, and they primary +// is to avoid relying on float64, and the primary // purpose is to have a predictable way to encode such // values used in query strings. -// Sign = true represents a negative value. // Scale is the number of digits to the right of the // decimal point. // Precision is currently not considered; precision, for // our purposes is implied to be the complete, known value. type Decimal struct { - Sign bool - Value uint32 + Value int64 Scale int64 } @@ -44,13 +42,10 @@ func (d Decimal) ToInt64(scale int64) int64 { var ret int64 scaleDiff := scale - d.Scale if scaleDiff == 0 { - ret = int64(d.Value) + ret = d.Value } else { ret = int64(float64(d.Value) * math.Pow10(int(scaleDiff))) } - if d.Sign { - ret *= -1 - } return ret } @@ -62,9 +57,6 @@ func (d Decimal) Float64() float64 { } else { ret = float64(d.Value) / math.Pow10(int(d.Scale)) } - if d.Sign { - ret *= -1 - } return ret } @@ -72,7 +64,16 @@ func (d Decimal) Float64() float64 { func (d Decimal) String() string { var s string + var neg bool sval := fmt.Sprintf("%d", d.Value) + + // Strip the negative sign off for now, and + // re-apply it at the end. + if sval[0] == '-' { + neg = true + sval = sval[1:] + } + if d.Scale == 0 { s = sval } else if d.Scale < 0 { @@ -103,7 +104,7 @@ func (d Decimal) String() string { s = string(buf) } - if d.Sign { + if neg { return "-" + s } return s @@ -118,7 +119,7 @@ const ( // ParseDecimal parses a string into a Decimal. func ParseDecimal(s string) (Decimal, error) { var sign bool - var value uint64 + var value int64 var scale int64 var err error @@ -218,14 +219,22 @@ func ParseDecimal(s string) (Decimal, error) { scale = 0 } - value, err = strconv.ParseUint(string(mantissa), 10, 32) + value, err = strconv.ParseInt(string(mantissa), 10, 64) if err != nil { return Decimal{}, errors.Wrap(err, "converting mantissa to uint32") } + // Because we pulled the sign off at the beginning, if value is + // negative here, it likely means the string had two "-"" characters. + if value < 0 { + return Decimal{}, errors.New("invalid negative value") + } + + if sign { + value *= -1 + } return Decimal{ - Sign: sign, - Value: uint32(value), + Value: value, Scale: scale, }, nil } diff --git a/pql/decimal_test.go b/pql/decimal_test.go index 4368767fe..6c6e2150b 100644 --- a/pql/decimal_test.go +++ b/pql/decimal_test.go @@ -29,42 +29,41 @@ func TestDecimal(t *testing.T) { exp pql.Decimal expErr string }{ - {"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}, ""}, + {"0", pql.Decimal{0, 0}, ""}, + {"-0", pql.Decimal{0, 0}, ""}, + {"0.0", pql.Decimal{0, 0}, ""}, + {"-0.00", pql.Decimal{0, 0}, ""}, + {"123.4567", pql.Decimal{1234567, 4}, ""}, + {" 123.4567", pql.Decimal{1234567, 4}, ""}, + {" 123.4567 ", pql.Decimal{1234567, 4}, ""}, + {"123.456700", pql.Decimal{1234567, 4}, ""}, + {"00123.4567", pql.Decimal{1234567, 4}, ""}, + {"+123.4567", pql.Decimal{1234567, 4}, ""}, + {"-123.4567", pql.Decimal{-1234567, 4}, ""}, + {"-00123.4567", pql.Decimal{-1234567, 4}, ""}, + {"-12.25", pql.Decimal{-1225, 2}, ""}, - {"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}, ""}, + {"123", pql.Decimal{123, 0}, ""}, + {"-12300", pql.Decimal{-123, -2}, ""}, + {"+012300", pql.Decimal{123, -2}, ""}, + {"12300", pql.Decimal{123, -2}, ""}, + {"12300.", pql.Decimal{123, -2}, ""}, + {"12300.0", pql.Decimal{123, -2}, ""}, + {"123.0", pql.Decimal{123, 0}, ""}, - {".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}, ""}, + {".123", pql.Decimal{123, 3}, ""}, + {"0.123", pql.Decimal{123, 3}, ""}, + {"0.001230", pql.Decimal{123, 5}, ""}, + {" 0.001230 ", pql.Decimal{123, 5}, ""}, + {"-0.001230 ", pql.Decimal{-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}, ""}, + // int64 edges. + {".000009223372036854775807", pql.Decimal{9223372036854775807, 24}, ""}, + {"-.000009223372036854775807", pql.Decimal{-9223372036854775807, 24}, ""}, + {"92233720368547.75807", pql.Decimal{9223372036854775807, 5}, ""}, + {"-92233720368547.75807", pql.Decimal{-9223372036854775807, 5}, ""}, + {"9223372036854775807000", pql.Decimal{9223372036854775807, -3}, ""}, + {"-9223372036854775807000", pql.Decimal{-9223372036854775807, -3}, ""}, // Error cases. {"", pql.Decimal{}, "decimal string is empty"}, @@ -72,20 +71,20 @@ func TestDecimal(t *testing.T) { {"*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"}, + {"--12300", pql.Decimal{}, "invalid negative value"}, + {"922337203685477580.8", pql.Decimal{}, "value out of range"}, + {"-922337203685477580.8", pql.Decimal{}, "value out of range"}, + {"9223372036854775808000", pql.Decimal{}, "value out of range"}, + {"-9223372036854775808000", pql.Decimal{}, "value out of range"}, } for i, test := range tests { dec, err := pql.ParseDecimal(test.s) 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) + t.Fatalf("test %d expected error to contain: %s, but got: %v", i, test.expErr, err) } } else if err != nil { - t.Fatalf("parsing string `%s`: %s", test.s, err) + t.Fatalf("test %d parsing string `%s`: %s", i, test.s, err) } else if dec != test.exp { t.Fatalf("test %d expected: %v, but got: %v", i, test.exp, dec) } @@ -98,22 +97,22 @@ func TestDecimal(t *testing.T) { scale int64 exp int64 }{ - {pql.Decimal{false, 0, 0}, 0, 0}, // 0 : 0 - {pql.Decimal{false, 0, 0}, 1, 0}, // 0 : 0.0 - {pql.Decimal{false, 0, 0}, -1, 0}, // 0 : 0 + {pql.Decimal{0, 0}, 0, 0}, // 0 : 0 + {pql.Decimal{0, 0}, 1, 0}, // 0 : 0.0 + {pql.Decimal{0, 0}, -1, 0}, // 0 : 0 - {pql.Decimal{false, 1234567, 4}, 5, 12345670}, // 123.4567 : 123.45670 - {pql.Decimal{false, 1234567, 4}, 4, 1234567}, // 123.4567 : 123.4567 - {pql.Decimal{false, 1234567, 4}, 3, 123456}, // 123.4567 : 123.456 + {pql.Decimal{1234567, 4}, 5, 12345670}, // 123.4567 : 123.45670 + {pql.Decimal{1234567, 4}, 4, 1234567}, // 123.4567 : 123.4567 + {pql.Decimal{1234567, 4}, 3, 123456}, // 123.4567 : 123.456 - {pql.Decimal{true, 1234567, 4}, 5, -12345670}, // -123.4567 : -123.45670 - {pql.Decimal{true, 1234567, 4}, 4, -1234567}, // -123.4567 : -123.4567 - {pql.Decimal{true, 1234567, 4}, 3, -123456}, // -123.4567 : -123.456 + {pql.Decimal{-1234567, 4}, 5, -12345670}, // -123.4567 : -123.45670 + {pql.Decimal{-1234567, 4}, 4, -1234567}, // -123.4567 : -123.4567 + {pql.Decimal{-1234567, 4}, 3, -123456}, // -123.4567 : -123.456 - {pql.Decimal{false, 123, -2}, 5, 1230000000}, // 12300 : 12300.00000 - {pql.Decimal{false, 123, -2}, -1, 1230}, // 12300 : 1230 - {pql.Decimal{false, 123, 1}, -1, 1}, // 12.3 : 1 - {pql.Decimal{false, 123, 1}, -2, 0}, // 12.3 : 0 + {pql.Decimal{123, -2}, 5, 1230000000}, // 12300 : 12300.00000 + {pql.Decimal{123, -2}, -1, 1230}, // 12300 : 1230 + {pql.Decimal{123, 1}, -1, 1}, // 12.3 : 1 + {pql.Decimal{123, 1}, -2, 0}, // 12.3 : 0 } for i, test := range tests { v := test.dec.ToInt64(test.scale) diff --git a/pql/parser_test.go b/pql/parser_test.go index 63b8e3cca..29c358865 100644 --- a/pql/parser_test.go +++ b/pql/parser_test.go @@ -105,10 +105,10 @@ func TestParser_Parse(t *testing.T) { &pql.Call{ Name: "Row", Args: map[string]interface{}{ - "key": pql.Decimal{false, 1225, 2}, - "foo": pql.Decimal{false, 13167, 3}, - "bar": pql.Decimal{false, 2, 0}, - "baz": pql.Decimal{false, 9, 1}, + "key": pql.Decimal{1225, 2}, + "foo": pql.Decimal{13167, 3}, + "bar": pql.Decimal{2, 0}, + "baz": pql.Decimal{9, 1}, }, }, ) { @@ -125,7 +125,7 @@ func TestParser_Parse(t *testing.T) { &pql.Call{ Name: "Row", Args: map[string]interface{}{ - "key": pql.Decimal{true, 1225, 2}, + "key": pql.Decimal{-1225, 2}, "foo": int64(-13), }, }, @@ -181,7 +181,7 @@ func TestParser_Parse(t *testing.T) { Name: "Row", Args: map[string]interface{}{ "key": "foo", - "x": &pql.Condition{Op: pql.EQ, Value: pql.Decimal{false, 1225, 2}}, + "x": &pql.Condition{Op: pql.EQ, Value: pql.Decimal{1225, 2}}, "y": &pql.Condition{Op: pql.GTE, Value: int64(100)}, "z": &pql.Condition{Op: pql.BETWEEN, Value: []interface{}{int64(4), int64(8)}}, "m": &pql.Condition{Op: pql.NEQ, Value: nil}, @@ -191,5 +191,4 @@ func TestParser_Parse(t *testing.T) { t.Fatalf("unexpected call: %#v", q.Calls[0]) } }) - }