Skip to content

Commit 261cae9

Browse files
committed
roachtest: fix unsortedMatricesDiffWithFloatComp helper
`unsortedMatricesDiffWithFloatComp` has a special logic for handling float and decimal arrays, and it figures out whether that logic is applicable based on the stringified type name. Previously, we would only match strings like `[]FLOAT4` for that, but lib/pq library returns `_FLOAT4` as the name for that array, so we would previously not apply the special logic in some cases, which would lead to spurious failures. This is now fixed. Release note: None
1 parent 2698603 commit 261cae9

File tree

2 files changed

+37
-11
lines changed

2 files changed

+37
-11
lines changed

pkg/cmd/roachtest/tests/query_comparison_util.go

+29-11
Original file line numberDiff line numberDiff line change
@@ -611,13 +611,30 @@ func joinAndSortRows(rowMatrix1, rowMatrix2 [][]string, sep string) (rows1, rows
611611
return rows1, rows2
612612
}
613613

614+
func isFloatArray(colType string) bool {
615+
switch colType {
616+
case "[]FLOAT4", "[]FLOAT8", "_FLOAT4", "_FLOAT8":
617+
return true
618+
default:
619+
return false
620+
}
621+
}
622+
623+
func isDecimalArray(colType string) bool {
624+
switch colType {
625+
case "[]DECIMAL", "_DECIMAL":
626+
return true
627+
default:
628+
return false
629+
}
630+
}
631+
614632
func needApproximateMatch(colType string) bool {
615633
// On s390x, check that values for both float and decimal coltypes are
616634
// approximately equal to take into account platform differences in floating
617635
// point calculations. On other architectures, check float values only.
618-
return (runtime.GOARCH == "s390x" && (colType == "DECIMAL" || colType == "[]DECIMAL")) ||
619-
colType == "FLOAT4" || colType == "[]FLOAT4" ||
620-
colType == "FLOAT8" || colType == "[]FLOAT8"
636+
return (runtime.GOARCH == "s390x" && (colType == "DECIMAL" || isDecimalArray(colType))) ||
637+
colType == "FLOAT4" || colType == "FLOAT8" || isFloatArray(colType)
621638
}
622639

623640
// sortRowsWithFloatComp is similar to joinAndSortRows, but it uses float
@@ -627,7 +644,7 @@ func sortRowsWithFloatComp(rowMatrix1, rowMatrix2 [][]string, colTypes []string)
627644
for idx := range colTypes {
628645
if needApproximateMatch(colTypes[idx]) {
629646
cmpFn := floatcmp.FloatsCmp
630-
if strings.HasPrefix(colTypes[idx], "[]") {
647+
if isFloatArray(colTypes[idx]) || isDecimalArray(colTypes[idx]) {
631648
cmpFn = floatcmp.FloatArraysCmp
632649
}
633650
res, err := cmpFn(rowMatrix[i][idx], rowMatrix[j][idx])
@@ -697,7 +714,7 @@ func unsortedMatricesDiffWithFloatComp(
697714
}
698715
var needCustomMatch bool
699716
for _, colType := range colTypes {
700-
if needApproximateMatch(colType) || colType == "DECIMAL" || colType == "[]DECIMAL" {
717+
if needApproximateMatch(colType) || colType == "DECIMAL" || isDecimalArray(colType) {
701718
needCustomMatch = true
702719
break
703720
}
@@ -712,13 +729,14 @@ func unsortedMatricesDiffWithFloatComp(
712729

713730
for j, colType := range colTypes {
714731
if needApproximateMatch(colType) {
732+
isFloatOrDecimalArray := isFloatArray(colType) || isDecimalArray(colType)
715733
cmpFn := floatcmp.FloatsMatch
716734
switch {
717-
case runtime.GOARCH == "s390x" && strings.HasPrefix(colType, "[]"):
735+
case runtime.GOARCH == "s390x" && isFloatOrDecimalArray:
718736
cmpFn = floatcmp.FloatArraysMatchApprox
719-
case runtime.GOARCH == "s390x" && !strings.HasPrefix(colType, "[]"):
737+
case runtime.GOARCH == "s390x" && !isFloatOrDecimalArray:
720738
cmpFn = floatcmp.FloatsMatchApprox
721-
case strings.HasPrefix(colType, "[]"):
739+
case isFloatOrDecimalArray:
722740
cmpFn = floatcmp.FloatArraysMatch
723741
}
724742
match, err := cmpFn(row1[j], row2[j])
@@ -729,12 +747,12 @@ func unsortedMatricesDiffWithFloatComp(
729747
return result, nil
730748
}
731749
} else {
732-
switch colType {
733-
case "DECIMAL":
750+
switch {
751+
case colType == "DECIMAL":
734752
// For decimals, remove any trailing zeroes.
735753
row1[j] = trimDecimalTrailingZeroes(row1[j])
736754
row2[j] = trimDecimalTrailingZeroes(row2[j])
737-
case "[]DECIMAL":
755+
case isDecimalArray(colType):
738756
// For decimal arrays, remove any trailing zeroes from each
739757
// decimal.
740758
row1[j] = trimDecimalsTrailingZeroesInArray(row1[j])

pkg/cmd/roachtest/tests/query_comparison_util_test.go

+8
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,14 @@ func TestUnsortedMatricesDiff(t *testing.T) {
148148
exactMatch: false,
149149
approxMatch: true,
150150
},
151+
{
152+
name: "multi row 0 in array matches -0 in array, lib/pq type name",
153+
colTypes: []string{"_FLOAT4"}, // this is how []FLOAT4 is named in lib/pq driver
154+
t1: [][]string{{"NULL"}, {"{-1}"}, {"{-0}"}, {"{0}"}, {"{NaN}"}},
155+
t2: [][]string{{"NULL"}, {"{-1}"}, {"{0}"}, {"{0}"}, {"{NaN}"}},
156+
exactMatch: false,
157+
approxMatch: true,
158+
},
151159
}
152160
for _, tc := range tcs {
153161
t.Run(tc.name, func(t *testing.T) {

0 commit comments

Comments
 (0)