tests: support float approximation in roachtest query comparison utils

[rharding6373]

tests, logictest, floatcmp: refactor comparison test util functions

This commit moves some float comparison test util functions from
logictest into the floatcmp package. It also moves a query result
comparison function from the tlp file to query_comparison_util in the
tests package.

This commit also marks roachtests as testonly targets.

Epic: none

Release note: None

tests: support float approximation in roachtest query comparison utils

Before this change unoptimized query oracle tests would compare results
using simple string comparison. However, due to floating point precision
limitations, it's possible for results with floating point to diverge
during the course of normal computation. This results in test failures
that are difficult to reproduce or determine whether they are expected
behavior.

This change utilizes existing floating point comparison functions used
by logic tests to match float values only to a specific precision. Like
the logic tests, we also have special handling for floats and decimals
under the s390x architecture (see #63244). In order to avoid costly
comparisons, we only check floating point precision if the naiive string
comparison approach fails and there are float or decimal types in the
result.

Epic: None
Fixes: #95665

Release note: None

[cockroach-teamcity]

This change is 

[michae2]

Thank you for doing this!

Reviewed 9 of 9 files at r1, 2 of 4 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @herkolategan, @renatolabs, and @rharding6373)

---

pkg/cmd/roachtest/BUILD.bazel line 73 at r1 (raw file):

    name = "roachtest_test",
    size = "small",
    testonly = 1,

TIL about testonly, nice!

---

pkg/cmd/roachtest/tests/query_comparison_util.go line 406 at r2 (raw file):

	// comparison, so that we can split the sorted rows if we need to make
	// additional comparisons.
	sep := ",unsortedMatricesDiffWithFloatComp separator,"

This is a nice technique for splitting again, but won't it also show up in the error message? Maybe the reported diff should be calculated again, without this separator, to make it easier to read?

---

pkg/cmd/roachtest/tests/unoptimized_query_oracle.go line 177 at r2 (raw file):

		return nil
	}
	diff, err := unsortedMatricesDiffWithFloatComp(unoptimizedRows, optimizedRows, h.colTypes)

Nice! I think we also want this in costfuzz and tlp, right?

---

pkg/testutils/floatcmp/floatcmp.go line 127 at r1 (raw file):

	// 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) {

I know this is how the code was before, but I think it would be more accurate to use math.Frexp to get the power-of-2 normalization. (I think the power-of-10 normalization might itself introduce floating-point error.)

[rharding6373]

TFTR! PTAL

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @herkolategan, @michae2, and @renatolabs)

---

pkg/cmd/roachtest/tests/query_comparison_util.go line 406 at r2 (raw file):

Previously, michae2 (Michael Erickson) wrote…

This is a nice technique for splitting again, but won't it also show up in the error message? Maybe the reported diff should be calculated again, without this separator, to make it easier to read?

You're right. I redid the join/sort if there is an error and there are floats instead.

---

pkg/cmd/roachtest/tests/unoptimized_query_oracle.go line 177 at r2 (raw file):

Previously, michae2 (Michael Erickson) wrote…

Nice! I think we also want this in costfuzz and tlp, right?

I modified costfuzz to use this. For tlp, we expect a diff because we already did a sql comparison that failed, so I think that using the old diff method is ok.

---

pkg/testutils/floatcmp/floatcmp.go line 127 at r1 (raw file):

Previously, michae2 (Michael Erickson) wrote…

I know this is how the code was before, but I think it would be more accurate to use math.Frexp to get the power-of-2 normalization. (I think the power-of-10 normalization might itself introduce floating-point error.)

Done.

[michae2]

Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @herkolategan, @renatolabs, and @rharding6373)

---

pkg/cmd/roachtest/tests/query_comparison_util.go line 478 at r3 (raw file):

		}
	}
	return "", nil

Do we also need to confirm that the rest of the columns are equal? (In other words, if the result contains float columns, and those are equal, but some other non-float column is not equal, will this catch that?)

It might be good to add some testcases with both float and non-float columns.

---

pkg/cmd/roachtest/tests/query_comparison_util_test.go line 56 at r3 (raw file):

		{
			name:        "multi float approx match",
			colTypes:    []string{"FLOAT8"},

Should this slice have two values?

[rharding6373]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @herkolategan, @michae2, and @renatolabs)

---

pkg/cmd/roachtest/tests/query_comparison_util.go line 478 at r3 (raw file):

Previously, michae2 (Michael Erickson) wrote…

Do we also need to confirm that the rest of the columns are equal? (In other words, if the result contains float columns, and those are equal, but some other non-float column is not equal, will this catch that?)

It might be good to add some testcases with both float and non-float columns.

Done.

---

pkg/cmd/roachtest/tests/query_comparison_util_test.go line 56 at r3 (raw file):

Previously, michae2 (Michael Erickson) wrote…

Should this slice have two values?

Done.

[michae2]

Nice work!

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @herkolategan and @renatolabs)

[rharding6373]

The test failure is unrelated. I opened #107455 to address it.

TFTRs!

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)