Skip to content

Commit

Permalink
Merge #55530
Browse files Browse the repository at this point in the history
55530: logictest: introduce a separate matcher for floats r=yuzefovich a=yuzefovich

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.

Informs: #55268.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
craig[bot] and yuzefovich committed Oct 15, 2020
2 parents 00d70ee + 9d2c054 commit 5403e6b
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 23 deletions.
138 changes: 117 additions & 21 deletions pkg/sql/logictest/logic.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"go/build"
"io"
"math"
"math/rand"
"net/url"
"os"
Expand Down Expand Up @@ -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
//
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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 != "" {
Expand All @@ -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)
Expand Down
29 changes: 29 additions & 0 deletions pkg/sql/logictest/logic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/crdb_internal
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/float
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/select
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5403e6b

Please sign in to comment.