logictest: compare floating point values approximately on s390x

pkg/BUILD.bazel

@@ -265,6 +265,7 @@ ALL_TESTS = [
     "//pkg/storage/enginepb:enginepb_test",
     "//pkg/storage/metamorphic:metamorphic_test",
     "//pkg/storage:storage_test",
+    "//pkg/testutils/floatcmp:floatcmp_test",
     "//pkg/testutils/keysutils:keysutils_test",
     "//pkg/testutils/lint/passes/fmtsafe:fmtsafe_test",
     "//pkg/testutils/lint/passes/forbiddenmethod:forbiddenmethod_test",

pkg/sql/logictest/BUILD.bazel

@@ -32,6 +32,7 @@ go_library(
         "//pkg/sql/sessiondata",
         "//pkg/sql/stats",
         "//pkg/testutils",
+        "//pkg/testutils/floatcmp",
         "//pkg/testutils/physicalplanutils",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/skip",

pkg/sql/logictest/logic.go

@@ -27,6 +27,7 @@ import (
 	"path/filepath"
 	"reflect"
 	"regexp"
+	"runtime"
 	"runtime/debug"
 	"runtime/trace"
 	"sort"
@@ -57,6 +58,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/stats"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/floatcmp"
 	"github.com/cockroachdb/cockroach/pkg/testutils/physicalplanutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
@@ -2832,9 +2834,16 @@ func (t *logicTest) execQuery(query logicQuery) error {
 			// To find the coltype for the given result, mod the result number
 			// by the number of coltypes.
 			colT := query.colTypes[i%len(query.colTypes)]
-			if !resultMatches && colT == 'F' {
+			if !resultMatches {
 				var err error
-				resultMatches, err = floatsMatch(expected, actual)
+				// On s390x, check that values for both float ('F') and decimal
+				// ('R') coltypes are approximately equal to take into account
+				// platform differences in floating point calculations.
+				if runtime.GOARCH == "s390x" && (colT == 'F' || colT == 'R') {
+					resultMatches, err = floatsMatchApprox(expected, actual)
+				} else if colT == 'F' {
+					resultMatches, err = floatsMatch(expected, actual)
+				}
 				if err != nil {
 					return errors.CombineErrors(makeError(), err)
 				}
@@ -2892,17 +2901,37 @@ 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) {
+// parseExpectedAndActualFloats converts the strings expectedString and
+// actualString to float64 values.
+func parseExpectedAndActualFloats(expectedString, actualString string) (float64, float64, error) {
 	expected, err := strconv.ParseFloat(expectedString, 64 /* bitSize */)
 	if err != nil {
-		return false, errors.Wrap(err, "when parsing expected")
+		return 0, 0, errors.Wrap(err, "when parsing expected")
 	}
 	actual, err := strconv.ParseFloat(actualString, 64 /* bitSize */)
 	if err != nil {
-		return false, errors.Wrap(err, "when parsing actual")
+		return 0, 0, errors.Wrap(err, "when parsing actual")
+	}
+	return expected, actual, nil
+}
+
+// floatsMatchApprox returns whether two floating point represented as
+// strings are equal within a tolerance.
+func floatsMatchApprox(expectedString, actualString string) (bool, error) {
+	expected, actual, err := parseExpectedAndActualFloats(expectedString, actualString)
+	if err != nil {
+		return false, err
+	}
+	return floatcmp.EqualApprox(expected, actual, floatcmp.CloseFraction, floatcmp.CloseMargin), 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, actual, err := parseExpectedAndActualFloats(expectedString, actualString)
+	if err != nil {
+		return false, err
 	}
 	// Check special values - NaN, +Inf, -Inf, 0.
 	if math.IsNaN(expected) || math.IsNaN(actual) {

pkg/sql/logictest/logic_test.go

@@ -36,7 +36,8 @@ func TestSqlLiteLogic(t *testing.T) {
 	RunSQLLiteLogicTest(t, "" /* configOverride */)
 }
 
-// TestFloatsMatch is a unit test for floatsMatch() function.
+// TestFloatsMatch is a unit test for floatsMatch() and floatsMatchApprox()
+// functions.
 func TestFloatsMatch(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	for _, tc := range []struct {
@@ -60,7 +61,15 @@ func TestFloatsMatch(t *testing.T) {
 			t.Fatal(err)
 		}
 		if match != tc.match {
-			t.Fatalf("wrong result on %v", tc)
+			t.Fatalf("floatsMatch: wrong result on %v", tc)
+		}
+
+		match, err = floatsMatchApprox(tc.f1, tc.f2)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if match != tc.match {
+			t.Fatalf("floatsMatchApprox: wrong result on %v", tc)
 		}
 	}
 }

pkg/testutils/floatcmp/BUILD.bazel

@@ -0,0 +1,19 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "floatcmp",
+    srcs = ["floatcmp.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/testutils/floatcmp",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@com_github_google_go_cmp//cmp",
+        "@com_github_google_go_cmp//cmp/cmpopts",
+    ],
+)
+
+go_test(
+    name = "floatcmp_test",
+    size = "small",
+    srcs = ["floatcmp_test.go"],
+    embed = [":floatcmp"],
+)

pkg/testutils/floatcmp/floatcmp.go

@@ -0,0 +1,77 @@
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+// Package floatcmp provides functions for determining float values to be equal
+// if they are within a tolerance. It is designed to be used in tests.
+package floatcmp
+
+import (
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+)
+
+const (
+	// CloseFraction can be used to set a "close" tolerance for the fraction
+	// argument of functions in this package. It should typically be used with
+	// the CloseMargin constant for the margin argument. Its value is taken from
+	// the close tolerances in go's math package.
+	CloseFraction float64 = 1e-14
+
+	// CloseMargin can be used to set a "close" tolerance for the margin
+	// argument of functions in this package. It should typically be used with
+	// the CloseFraction constant for the fraction argument.
+	//
+	// It is set to the square of CloseFraction so it is only used when the
+	// smaller of the absolute expected and actual values is in the range:
+	//
+	//   -CloseFraction <= 0 <= CloseFraction
+	//
+	// CloseMargin is greater than 0 otherwise if either expected or actual were
+	// 0 the calculated tolerance from the fraction would be 0.
+	CloseMargin float64 = CloseFraction * CloseFraction
+)
+
+// EqualApprox reports whether expected and actual are deeply equal with the
+// following modifications for float64 and float32 types:
+//
+// • If both expected and actual are not NaN or infinate, they are equal within
+// the larger of the relative fraction or absolute margin calculated from the
+// fraction and margin arguments.
+//
+// • If both expected and actual are NaN, they are equal.
+//
+// Both fraction and margin must be non-negative.
+//
+// fraction is used to calculate the tolerance as a relative fraction of the
+// smaller of expected and actual:
+//
+//   tolerance_frac = (fraction * min(|expected|, |actual|))
+//
+// margin specifies the tolerance as an absolute value:
+//
+//   tolerance_marg = margin
+//
+// The tolerance used to determine approximate equality is:
+//
+//   tolerance = max(tolerance_frac, tolerance_marg)
+//
+// To use only one of fraction or margin, set the other to 0.
+//
+// For comparing expected and actual values in tests, typically the fraction
+// should be set to the smallest relative fraction to tolerate. The margin
+// should be set to a much smaller value so that it is only used when:
+//
+//   (fraction * min(|expected|, |actual|)) < margin
+//
+// which allows expected and actual to be approximately equal within margin when
+// either is 0.
+func EqualApprox(expected interface{}, actual interface{}, fraction float64, margin float64) bool {
+	return cmp.Equal(expected, actual, cmpopts.EquateApprox(fraction, margin), cmpopts.EquateNaNs())
+}

pkg/testutils/floatcmp/floatcmp_test.go

@@ -0,0 +1,158 @@
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package floatcmp
+
+import (
+	"math"
+	"testing"
+)
+
+// EqualApprox takes an interface, allowing it to compare equality of both
+// primitive data types and structs. We want to test both cases.
+type (
+	// floatArgs holds the expected and actual values of floating point equality tests.
+	floatArgs struct {
+		expected float64
+		actual   float64
+	}
+
+	// testStruct structs are the values compared in struct equality tests.
+	testStruct struct {
+		X, Y float64
+		I    int
+	}
+
+	// structArgs holds the expected and actual values of struct equality tests.
+	structArgs struct {
+		expected testStruct
+		actual   testStruct
+	}
+
+	// floatTestCase represents a test case for floating point values.
+	floatTestCase struct {
+		name string
+		args floatArgs
+		want bool
+	}
+
+	// structTestCase represents a test case for struct values.
+	structTestCase struct {
+		name string
+		args structArgs
+		want bool
+	}
+)
+
+var floatTests = []floatTestCase{
+	{
+		name: "zeros",
+		args: floatArgs{expected: 0, actual: 0},
+		want: true,
+	},
+	{
+		name: "NaNs",
+		args: floatArgs{expected: math.NaN(), actual: math.NaN()},
+		want: true,
+	},
+	{
+		name: "zero not close to NaN",
+		args: floatArgs{expected: 0, actual: math.NaN()},
+		want: false,
+	},
+	{
+		name: "positive infinities",
+		args: floatArgs{expected: math.Inf(+1), actual: math.Inf(+1)},
+		want: true,
+	},
+	{
+		name: "negative infinities",
+		args: floatArgs{expected: math.Inf(-1), actual: math.Inf(-1)},
+		want: true,
+	},
+	{
+		name: "ones",
+		args: floatArgs{expected: 1, actual: 1},
+		want: true,
+	},
+	{
+		name: "signs",
+		args: floatArgs{expected: 1, actual: -1},
+		want: false,
+	},
+	{
+		name: "different",
+		args: floatArgs{expected: 1, actual: 2},
+		want: false,
+	},
+	{
+		name: "close to zero",
+		args: floatArgs{expected: 0, actual: math.Nextafter(0+CloseMargin, math.Inf(-1))},
+		want: true,
+	},
+	{
+		name: "not close to zero",
+		args: floatArgs{expected: 0, actual: math.Nextafter(0+CloseMargin, math.Inf(+1))},
+		want: false,
+	},
+	{
+		name: "close to CloseFraction",
+		args: floatArgs{expected: CloseFraction, actual: math.Nextafter(CloseFraction+CloseMargin, math.Inf(-1))},
+		want: true,
+	},
+	{
+		name: "not close to CloseFraction",
+		args: floatArgs{expected: CloseFraction, actual: math.Nextafter(CloseFraction+CloseMargin, math.Inf(+1))},
+		want: false,
+	},
+	{
+		name: "close to one",
+		args: floatArgs{expected: 1, actual: math.Nextafter(1+1*CloseFraction, math.Inf(-1))},
+		want: true,
+	},
+	{
+		name: "not close to one",
+		args: floatArgs{expected: 1, actual: math.Nextafter(1+1*CloseFraction, math.Inf(+1))},
+		want: false,
+	},
+}
+
+// toStructTests transforms an array of floatTestCases into an array of structTestCases by
+// copying the expected and actual values of each floatTestCase into corresponding values
+// in each structTestCase.
+func toStructTests(floatTestCases []floatTestCase) []structTestCase {
+	structTestCases := make([]structTestCase, 0, len(floatTestCases))
+	for _, ft := range floatTestCases {
+		structTestCases = append(structTestCases,
+			structTestCase{
+				name: ft.name + " struct",
+				args: structArgs{expected: testStruct{X: ft.args.expected, Y: ft.args.expected, I: 0}, actual: testStruct{X: ft.args.actual, Y: ft.args.actual, I: 0}},
+				want: ft.want,
+			})
+	}
+	return structTestCases
+}
+
+func TestEqualClose(t *testing.T) {
+	for _, tt := range floatTests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := EqualApprox(tt.args.expected, tt.args.actual, CloseFraction, CloseMargin); got != tt.want {
+				t.Errorf("Close(%.16e, %.16e) = %v, want %v", tt.args.expected, tt.args.actual, got, tt.want)
+			}
+		})
+	}
+	for _, tt := range toStructTests(floatTests) {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := EqualApprox(tt.args.expected, tt.args.actual, CloseFraction, CloseMargin); got != tt.want {
+				t.Errorf("Close(%.v, %.v) = %v, want %v", tt.args.expected, tt.args.actual, got, tt.want)
+			}
+		})
+	}
+}