Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-22.2: tests: support float approximation in roachtest query comparison utils #110291

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/cmd/roachtest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")

go_library(
name = "roachtest_lib",
testonly = 1,
srcs = [
"cluster.go",
"github.go",
Expand Down Expand Up @@ -58,13 +59,15 @@ go_library(

go_binary(
name = "roachtest",
testonly = 1,
embed = [":roachtest_lib"],
visibility = ["//visibility:public"],
)

go_test(
name = "roachtest_test",
size = "small",
testonly = 1,
srcs = [
"cluster_test.go",
"github_test.go",
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/roachtest/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "tests",
testonly = 1,
srcs = [
"acceptance.go",
"activerecord.go",
Expand Down Expand Up @@ -193,6 +194,9 @@ go_library(
"//pkg/sql/pgwire/pgerror",
"//pkg/testutils",
"//pkg/testutils/skip",
"//pkg/testutils/floatcmp",
"//pkg/testutils/jobutils",
"//pkg/testutils/release",
"//pkg/testutils/sqlutils",
"//pkg/ts/tspb",
"//pkg/util",
Expand Down Expand Up @@ -248,6 +252,7 @@ go_test(
srcs = [
"blocklist_test.go",
"drt_test.go",
"query_comparison_util_test.go",
"tpcc_test.go",
"util_load_group_test.go",
":mocks_drt", # keep
Expand All @@ -260,6 +265,7 @@ go_test(
"//pkg/roachprod/logger",
"//pkg/roachprod/prometheus",
"//pkg/testutils/skip",
"//pkg/util/leaktest",
"//pkg/util/version",
"@com_github_golang_mock//gomock",
"@com_github_google_go_github//github",
Expand Down
6 changes: 5 additions & 1 deletion pkg/cmd/roachtest/tests/costfuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ func runCostFuzzQuery(smither *sqlsmith.Smither, rnd *rand.Rand, h queryComparis
return nil
}

if diff := unsortedMatricesDiff(controlRows, perturbRows); diff != "" {
diff, err := unsortedMatricesDiffWithFloatComp(controlRows, perturbRows, h.colTypes)
if err != nil {
return err
}
if diff != "" {
// We have a mismatch in the perturbed vs control query outputs.
h.logStatements()
h.logVerboseOutput()
Expand Down
117 changes: 117 additions & 0 deletions pkg/cmd/roachtest/tests/query_comparison_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"math/rand"
"os"
"path/filepath"
"runtime"
"sort"
"strings"
"time"

Expand All @@ -26,9 +28,11 @@ import (
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
"github.com/cockroachdb/cockroach/pkg/internal/sqlsmith"
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
"github.com/cockroachdb/cockroach/pkg/testutils/floatcmp"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/randutil"
"github.com/cockroachdb/errors"
"github.com/google/go-cmp/cmp"
)

type queryComparisonTest struct {
Expand Down Expand Up @@ -285,6 +289,7 @@ type queryComparisonHelper struct {

statements []string
statementsAndExplains []sqlAndOutput
colTypes []string
}

// runQuery runs the given query and returns the output. As a side effect, it
Expand All @@ -297,6 +302,14 @@ func (h *queryComparisonHelper) runQuery(stmt string) ([][]string, error) {
return nil, err
}
defer rows.Close()
cts, err := rows.ColumnTypes()
if err != nil {
return nil, err
}
h.colTypes = make([]string, len(cts))
for i, ct := range cts {
h.colTypes[i] = ct.DatabaseTypeName()
}
return sqlutils.RowsToStrMatrix(rows)
}

Expand Down Expand Up @@ -350,3 +363,107 @@ func (h *queryComparisonHelper) logVerboseOutput() {
func (h *queryComparisonHelper) makeError(err error, msg string) error {
return errors.Wrapf(err, "%s. %d statements run", msg, h.stmtNo)
}

func joinAndSortRows(rowMatrix1, rowMatrix2 [][]string, sep string) (rows1, rows2 []string) {
for _, row := range rowMatrix1 {
rows1 = append(rows1, strings.Join(row[:], sep))
}
for _, row := range rowMatrix2 {
rows2 = append(rows2, strings.Join(row[:], sep))
}
sort.Strings(rows1)
sort.Strings(rows2)
return rows1, rows2
}

// unsortedMatricesDiffWithFloatComp sorts and compares the rows in rowMatrix1
// to rowMatrix2 and outputs a diff or message related to the comparison. If a
// string comparison of the rows fails, and they contain floats or decimals, it
// performs an approximate comparison of the values.
func unsortedMatricesDiffWithFloatComp(
rowMatrix1, rowMatrix2 [][]string, colTypes []string,
) (string, error) {
rows1, rows2 := joinAndSortRows(rowMatrix1, rowMatrix2, ",")
result := cmp.Diff(rows1, rows2)
if result == "" {
return result, nil
}
if len(rows1) != len(rows2) || len(colTypes) != len(rowMatrix1[0]) || len(colTypes) != len(rowMatrix2[0]) {
return result, nil
}
var needApproxMatch bool
for i := range colTypes {
// On s390x, check that values for both float and decimal coltypes are
// approximately equal to take into account platform differences in floating
// point calculations. On other architectures, check float values only.
if (runtime.GOARCH == "s390x" && colTypes[i] == "DECIMAL") ||
colTypes[i] == "FLOAT4" || colTypes[i] == "FLOAT8" {
needApproxMatch = true
break
}
}
if !needApproxMatch {
return result, nil
}
// Use an unlikely string as a separator so that we can make a comparison
// using sorted rows. We don't use the rows sorted above because splitting
// the rows could be ambiguous.
sep := ",unsortedMatricesDiffWithFloatComp separator,"
rows1, rows2 = joinAndSortRows(rowMatrix1, rowMatrix2, sep)
for i := range rows1 {
// Split the sorted rows.
row1 := strings.Split(rows1[i], sep)
row2 := strings.Split(rows2[i], sep)

for j := range row1 {
if runtime.GOARCH == "s390x" && colTypes[j] == "DECIMAL" {
// On s390x, check that values for both float and decimal coltypes are
// approximately equal to take into account platform differences in floating
// point calculations. On other architectures, check float values only.
match, err := floatcmp.FloatsMatchApprox(row1[j], row2[j])
if err != nil {
return "", err
}
if !match {
return result, nil
}
} else if colTypes[j] == "FLOAT4" || colTypes[j] == "FLOAT8" {
// Check that float values are approximately equal.
var err error
var match bool
if runtime.GOARCH == "s390x" {
match, err = floatcmp.FloatsMatchApprox(row1[j], row2[j])
} else {
match, err = floatcmp.FloatsMatch(row1[j], row2[j])
}
if err != nil {
return "", err
}
if !match {
return result, nil
}
} else {
// Check that other columns are equal with a string comparison.
if row1[j] != row2[j] {
return result, nil
}
}
}
}
return "", nil
}

// unsortedMatricesDiff sorts and compares rows of data.
func unsortedMatricesDiff(rowMatrix1, rowMatrix2 [][]string) string {
var rows1 []string
for _, row := range rowMatrix1 {
rows1 = append(rows1, strings.Join(row[:], ","))
}
var rows2 []string
for _, row := range rowMatrix2 {
rows2 = append(rows2, strings.Join(row[:], ","))
}
sort.Strings(rows1)
sort.Strings(rows2)
return cmp.Diff(rows1, rows2)
}
140 changes: 140 additions & 0 deletions pkg/cmd/roachtest/tests/query_comparison_util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
// Copyright 2023 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 tests

import (
"testing"

"github.com/cockroachdb/cockroach/pkg/util/leaktest"
)

// TestUnsortedMatricesDiff is a unit test for the
// unsortedMatricesDiffWithFloatComp() and unsortedMatricesDiff() utility
// functions.
func TestUnsortedMatricesDiff(t *testing.T) {
defer leaktest.AfterTest(t)()
tcs := []struct {
name string
colTypes []string
t1, t2 [][]string
exactMatch bool
approxMatch bool
}{
{
name: "float exact match",
colTypes: []string{"FLOAT8"},
t1: [][]string{{"1.2345678901234567"}},
t2: [][]string{{"1.2345678901234567"}},
exactMatch: true,
},
{
name: "float approx match",
colTypes: []string{"FLOAT8"},
t1: [][]string{{"1.2345678901234563"}},
t2: [][]string{{"1.2345678901234564"}},
exactMatch: false,
approxMatch: true,
},
{
name: "float no match",
colTypes: []string{"FLOAT8"},
t1: [][]string{{"1.234567890123"}},
t2: [][]string{{"1.234567890124"}},
exactMatch: false,
approxMatch: false,
},
{
name: "multi float approx match",
colTypes: []string{"FLOAT8", "FLOAT8"},
t1: [][]string{{"1.2345678901234567", "1.2345678901234567"}},
t2: [][]string{{"1.2345678901234567", "1.2345678901234568"}},
exactMatch: false,
approxMatch: true,
},
{
name: "string no match",
colTypes: []string{"STRING"},
t1: [][]string{{"hello"}},
t2: [][]string{{"world"}},
exactMatch: false,
approxMatch: false,
},
{
name: "mixed types match",
colTypes: []string{"STRING", "FLOAT8"},
t1: [][]string{{"hello", "1.2345678901234567"}},
t2: [][]string{{"hello", "1.2345678901234567"}},
exactMatch: true,
},
{
name: "mixed types float approx match",
colTypes: []string{"STRING", "FLOAT8"},
t1: [][]string{{"hello", "1.23456789012345678"}},
t2: [][]string{{"hello", "1.23456789012345679"}},
exactMatch: false,
approxMatch: true,
},
{
name: "mixed types no match",
colTypes: []string{"STRING", "FLOAT8"},
t1: [][]string{{"hello", "1.2345678901234567"}},
t2: [][]string{{"world", "1.2345678901234567"}},
exactMatch: false,
approxMatch: false,
},
{
name: "different col count",
colTypes: []string{"STRING"},
t1: [][]string{{"hello", "1.2345678901234567"}},
t2: [][]string{{"world", "1.2345678901234567"}},
exactMatch: false,
approxMatch: false,
},
{
name: "different row count",
colTypes: []string{"STRING", "FLOAT8"},
t1: [][]string{{"hello", "1.2345678901234567"}, {"aloha", "2.345"}},
t2: [][]string{{"world", "1.2345678901234567"}},
exactMatch: false,
approxMatch: false,
},
{
name: "multi row unsorted",
colTypes: []string{"STRING", "FLOAT8"},
t1: [][]string{{"hello", "1.2345678901234567"}, {"world", "1.2345678901234560"}},
t2: [][]string{{"world", "1.2345678901234560"}, {"hello", "1.2345678901234567"}},
exactMatch: true,
},
}
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
match := unsortedMatricesDiff(tc.t1, tc.t2)
if tc.exactMatch && match != "" {
t.Fatalf("unsortedMatricesDiff: expected exact match, got diff: %s", match)
} else if !tc.exactMatch && match == "" {
t.Fatalf("unsortedMatricesDiff: expected no exact match, got no diff")
}

var err error
match, err = unsortedMatricesDiffWithFloatComp(tc.t1, tc.t2, tc.colTypes)
if err != nil {
t.Fatal(err)
}
if tc.exactMatch && match != "" {
t.Fatalf("unsortedMatricesDiffWithFloatComp: expected exact match, got diff: %s", match)
} else if !tc.exactMatch && tc.approxMatch && match != "" {
t.Fatalf("unsortedMatricesDiffWithFloatComp: expected approx match, got diff: %s", match)
} else if !tc.exactMatch && !tc.approxMatch && match == "" {
t.Fatalf("unsortedMatricesDiffWithFloatComp: expected no approx match, got no diff")
}
})
}
}
Loading