From 9d2c054fd3e9e8c6a7d73f4e464ebd32ee82b8a2 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Tue, 13 Oct 2020 14:46:09 -0700 Subject: [PATCH] logictest: introduce a separate matcher for floats This commit adds another type annotation `F` intended to be used for floating point numbers which have a separate matcher that allows for some deviation in the precision. Namely, the matcher checks that the expected and the actual numbers match in 15 significant decimal digits. Note that I think the matching algorithm introduces some rounding errors, so it might not work perfectly in all scenarios, yet it should be good enough in rare test cases when we need it. Release note: None --- pkg/sql/logictest/logic.go | 138 +++++++++++++++--- pkg/sql/logictest/logic_test.go | 29 ++++ .../testdata/logic_test/crdb_internal | 2 +- pkg/sql/logictest/testdata/logic_test/float | 7 + pkg/sql/logictest/testdata/logic_test/select | 2 +- 5 files changed, 155 insertions(+), 23 deletions(-) diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index ec94b84f8650..d2d7553bcbdb 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -20,6 +20,7 @@ import ( "fmt" "go/build" "io" + "math" "math/rand" "net/url" "os" @@ -163,7 +164,9 @@ import ( // - T for text; also used for various types which get converted // to string (arrays, timestamps, etc.). // - I for integer -// - R for floating point or decimal +// - F for floating point (matches 15 significant decimal digits, +// https://www.postgresql.org/docs/9.0/datatype-numeric.html) +// - R for decimal // - B for boolean // - O for oid // @@ -2497,11 +2500,11 @@ func (t *logicTest) execQuery(query logicQuery) error { query.pos, i, val, val, ) } - case 'R': + case 'F', 'R': if valT != reflect.Float64 && valT != reflect.Slice { if *flexTypes && (valT == reflect.Int64) { t.signalIgnoredError( - fmt.Errorf("result type mismatch: expected R, got %T", val), query.pos, query.sql, + fmt.Errorf("result type mismatch: expected F or R, got %T", val), query.pos, query.sql, ) return nil } @@ -2583,7 +2586,14 @@ func (t *logicTest) execQuery(query logicQuery) error { return fmt.Errorf("%s: expected %d results, but found %d", query.pos, query.expectedValues, n) } if query.expectedHash != hash { - return fmt.Errorf("%s: expected %s, but found %s", query.pos, query.expectedHash, hash) + var suffix string + for _, colT := range query.colTypes { + if colT == 'F' { + suffix = "\tthis might be due to floating numbers precision deviation" + break + } + } + return fmt.Errorf("%s: expected %s, but found %s%s", query.pos, query.expectedHash, hash, suffix) } } @@ -2613,25 +2623,47 @@ func (t *logicTest) execQuery(query logicQuery) error { return nil } - if query.checkResults && !reflect.DeepEqual(query.expectedResults, actualResults) { - var buf bytes.Buffer - fmt.Fprintf(&buf, "%s: %s\nexpected:\n", query.pos, query.sql) - for _, line := range query.expectedResultsRaw { - fmt.Fprintf(&buf, " %s\n", line) - } - sortMsg := "" - if query.sorter != nil { - // We performed an order-insensitive comparison of "actual" vs "expected" - // rows by sorting both, but we'll display the error with the expected - // rows in the order in which they were put in the file, and the actual - // rows in the order in which the query returned them. - sortMsg = " -> ignore the following ordering of rows" + if query.checkResults { + makeError := func() error { + var buf bytes.Buffer + fmt.Fprintf(&buf, "%s: %s\nexpected:\n", query.pos, query.sql) + for _, line := range query.expectedResultsRaw { + fmt.Fprintf(&buf, " %s\n", line) + } + sortMsg := "" + if query.sorter != nil { + // We performed an order-insensitive comparison of "actual" vs "expected" + // rows by sorting both, but we'll display the error with the expected + // rows in the order in which they were put in the file, and the actual + // rows in the order in which the query returned them. + sortMsg = " -> ignore the following ordering of rows" + } + fmt.Fprintf(&buf, "but found (query options: %q%s) :\n", query.rawOpts, sortMsg) + for _, line := range t.formatValues(actualResultsRaw, query.valsPerLine) { + fmt.Fprintf(&buf, " %s\n", line) + } + return errors.Newf("%s", buf.String()) } - fmt.Fprintf(&buf, "but found (query options: %q%s) :\n", query.rawOpts, sortMsg) - for _, line := range t.formatValues(actualResultsRaw, query.valsPerLine) { - fmt.Fprintf(&buf, " %s\n", line) + if len(query.expectedResults) == 0 || len(actualResults) == 0 { + if len(query.expectedResults) != len(actualResults) { + return makeError() + } + } else { + for i, colT := range query.colTypes { + expected, actual := query.expectedResults[i], actualResults[i] + resultMatches := expected == actual + if !resultMatches && colT == 'F' { + var err error + resultMatches, err = floatsMatch(expected, actual) + if err != nil { + return errors.CombineErrors(makeError(), err) + } + } + if !resultMatches { + return makeError() + } + } } - return errors.Newf("%s", buf.String()) } if query.label != "" { @@ -2648,6 +2680,70 @@ func (t *logicTest) execQuery(query logicQuery) error { return nil } +// floatsMatch returns whether two floating point numbers represented as +// strings have matching 15 significant decimal digits (this is the precision +// that Postgres supports for 'double precision' type). +func floatsMatch(expectedString, actualString string) (bool, error) { + expected, err := strconv.ParseFloat(expectedString, 64 /* bitSize */) + if err != nil { + return false, errors.Wrap(err, "when parsing expected") + } + actual, err := strconv.ParseFloat(actualString, 64 /* bitSize */) + if err != nil { + return false, errors.Wrap(err, "when parsing actual") + } + // Check special values - NaN, +Inf, -Inf, 0. + if math.IsNaN(expected) || math.IsNaN(actual) { + return math.IsNaN(expected) == math.IsNaN(actual), nil + } + if math.IsInf(expected, 0 /* sign */) || math.IsInf(actual, 0 /* sign */) { + bothNegativeInf := math.IsInf(expected, -1 /* sign */) == math.IsInf(actual, -1 /* sign */) + bothPositiveInf := math.IsInf(expected, 1 /* sign */) == math.IsInf(actual, 1 /* sign */) + return bothNegativeInf || bothPositiveInf, nil + } + if expected == 0 || actual == 0 { + return expected == actual, nil + } + // Check that the numbers have the same sign. + if expected*actual < 0 { + return false, nil + } + expected = math.Abs(expected) + actual = math.Abs(actual) + // Check that 15 significant digits match. We do so by normalizing the + // numbers and then checking one digit at a time. + // + // normalize converts f to base * 10**power representation where base is in + // [1.0, 10.0) range. + normalize := func(f float64) (base float64, power int) { + for f >= 10 { + f = f / 10 + power++ + } + for f < 1 { + f *= 10 + power-- + } + return f, power + } + var expPower, actPower int + expected, expPower = normalize(expected) + actual, actPower = normalize(actual) + if expPower != actPower { + return false, nil + } + for i := 0; i < 15; i++ { + expDigit := int(expected) + actDigit := int(actual) + if expDigit != actDigit { + return false, nil + } + expected -= (expected - float64(expDigit)) * 10 + actual -= (actual - float64(actDigit)) * 10 + } + return true, nil +} + func (t *logicTest) formatValues(vals []string, valsPerLine int) []string { var buf bytes.Buffer tw := tabwriter.NewWriter(&buf, 2, 1, 2, ' ', 0) diff --git a/pkg/sql/logictest/logic_test.go b/pkg/sql/logictest/logic_test.go index e073e323a532..6f48a63dca1e 100644 --- a/pkg/sql/logictest/logic_test.go +++ b/pkg/sql/logictest/logic_test.go @@ -35,3 +35,32 @@ func TestSqlLiteLogic(t *testing.T) { defer leaktest.AfterTest(t)() RunSQLLiteLogicTest(t, "" /* configOverride */) } + +// TestFloatsMatch is a unit test for floatsMatch() function. +func TestFloatsMatch(t *testing.T) { + defer leaktest.AfterTest(t)() + for _, tc := range []struct { + f1, f2 string + match bool + }{ + {f1: "NaN", f2: "+Inf", match: false}, + {f1: "+Inf", f2: "+Inf", match: true}, + {f1: "NaN", f2: "NaN", match: true}, + {f1: "+Inf", f2: "-Inf", match: false}, + {f1: "-0.0", f2: "0.0", match: true}, + {f1: "0.0", f2: "NaN", match: false}, + {f1: "123.45", f2: "12.345", match: false}, + {f1: "0.1234567890123456", f2: "0.1234567890123455", match: true}, + {f1: "0.1234567890123456", f2: "0.1234567890123457", match: true}, + {f1: "-0.1234567890123456", f2: "0.1234567890123456", match: false}, + {f1: "-0.1234567890123456", f2: "-0.1234567890123455", match: true}, + } { + match, err := floatsMatch(tc.f1, tc.f2) + if err != nil { + t.Fatal(err) + } + if match != tc.match { + t.Fatalf("wrong result on %v", tc) + } + } +} diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal b/pkg/sql/logictest/testdata/logic_test/crdb_internal index 7f9a59e879e9..e955802a8bc0 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal @@ -144,7 +144,7 @@ SELECT * FROM crdb_internal.leases WHERE node_id < 0 ---- node_id table_id name parent_id expiration deleted -query ITTTTIIITFFFFFFFFFFFFFFFFF colnames +query ITTTTIIITRRRRRRRRRRRRRRRRR colnames SELECT * FROM crdb_internal.node_statement_statistics WHERE node_id < 0 ---- node_id application_name flags key anonymized count first_attempt_count max_retries last_error rows_avg rows_var parse_lat_avg parse_lat_var plan_lat_avg plan_lat_var run_lat_avg run_lat_var service_lat_avg service_lat_var overhead_lat_avg overhead_lat_var bytes_read_avg bytes_read_var rows_read_avg rows_read_var implicit_txn diff --git a/pkg/sql/logictest/testdata/logic_test/float b/pkg/sql/logictest/testdata/logic_test/float index e881db4ace0e..7ec53f1b5c35 100644 --- a/pkg/sql/logictest/testdata/logic_test/float +++ b/pkg/sql/logictest/testdata/logic_test/float @@ -132,3 +132,10 @@ DROP TABLE vals statement ok RESET extra_float_digits + +# Test that floating point numbers are compared with 15 decimal digits +# precision. +query FFF +SELECT -0.1234567890123456, 123456789012345.6, 1234567.890123456 +---- +-0.1234567890123457 123456789012345.7 1234567.890123457 diff --git a/pkg/sql/logictest/testdata/logic_test/select b/pkg/sql/logictest/testdata/logic_test/select index fc2b5b326c20..e5b88db298bd 100644 --- a/pkg/sql/logictest/testdata/logic_test/select +++ b/pkg/sql/logictest/testdata/logic_test/select @@ -688,7 +688,7 @@ CREATE TABLE wide (id INT4 NOT NULL, a INT4, b VARCHAR(255), c INT4, d VARCHAR(2 statement ok INSERT INTO wide(id, n) VALUES(0, 10) -query IITITTITTTTIFFI +query IITITTITTTTIRRI SELECT * FROM wide ---- 0 NULL NULL NULL NULL NULL NULL NULL NULL NULL NULL NULL NULL NULL 10