Skip to content

Commit

Permalink
Merge pull request #140148 from cockroachdb/blathers/backport-release…
Browse files Browse the repository at this point in the history
…-24.3-139799

release-24.3: roachtest: fix unsortedMatricesDiffWithFloatComp helper (#140148)

Co-Authored-By: Yahor Yuzefovich <[email protected]>
  • Loading branch information
yuzefovich and yuzefovich authored Jan 31, 2025
2 parents 698daeb + d56b2e1 commit d0be0aa
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 11 deletions.
40 changes: 29 additions & 11 deletions pkg/cmd/roachtest/tests/query_comparison_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,13 +611,30 @@ func joinAndSortRows(rowMatrix1, rowMatrix2 [][]string, sep string) (rows1, rows
return rows1, rows2
}

func isFloatArray(colType string) bool {
switch colType {
case "[]FLOAT4", "[]FLOAT8", "_FLOAT4", "_FLOAT8":
return true
default:
return false
}
}

func isDecimalArray(colType string) bool {
switch colType {
case "[]DECIMAL", "_DECIMAL":
return true
default:
return false
}
}

func needApproximateMatch(colType string) bool {
// 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.
return (runtime.GOARCH == "s390x" && (colType == "DECIMAL" || colType == "[]DECIMAL")) ||
colType == "FLOAT4" || colType == "[]FLOAT4" ||
colType == "FLOAT8" || colType == "[]FLOAT8"
return (runtime.GOARCH == "s390x" && (colType == "DECIMAL" || isDecimalArray(colType))) ||
colType == "FLOAT4" || colType == "FLOAT8" || isFloatArray(colType)
}

// sortRowsWithFloatComp is similar to joinAndSortRows, but it uses float
Expand All @@ -627,7 +644,7 @@ func sortRowsWithFloatComp(rowMatrix1, rowMatrix2 [][]string, colTypes []string)
for idx := range colTypes {
if needApproximateMatch(colTypes[idx]) {
cmpFn := floatcmp.FloatsCmp
if strings.HasPrefix(colTypes[idx], "[]") {
if isFloatArray(colTypes[idx]) || isDecimalArray(colTypes[idx]) {
cmpFn = floatcmp.FloatArraysCmp
}
res, err := cmpFn(rowMatrix[i][idx], rowMatrix[j][idx])
Expand Down Expand Up @@ -697,7 +714,7 @@ func unsortedMatricesDiffWithFloatComp(
}
var needCustomMatch bool
for _, colType := range colTypes {
if needApproximateMatch(colType) || colType == "DECIMAL" || colType == "[]DECIMAL" {
if needApproximateMatch(colType) || colType == "DECIMAL" || isDecimalArray(colType) {
needCustomMatch = true
break
}
Expand All @@ -712,13 +729,14 @@ func unsortedMatricesDiffWithFloatComp(

for j, colType := range colTypes {
if needApproximateMatch(colType) {
isFloatOrDecimalArray := isFloatArray(colType) || isDecimalArray(colType)
cmpFn := floatcmp.FloatsMatch
switch {
case runtime.GOARCH == "s390x" && strings.HasPrefix(colType, "[]"):
case runtime.GOARCH == "s390x" && isFloatOrDecimalArray:
cmpFn = floatcmp.FloatArraysMatchApprox
case runtime.GOARCH == "s390x" && !strings.HasPrefix(colType, "[]"):
case runtime.GOARCH == "s390x" && !isFloatOrDecimalArray:
cmpFn = floatcmp.FloatsMatchApprox
case strings.HasPrefix(colType, "[]"):
case isFloatOrDecimalArray:
cmpFn = floatcmp.FloatArraysMatch
}
match, err := cmpFn(row1[j], row2[j])
Expand All @@ -729,12 +747,12 @@ func unsortedMatricesDiffWithFloatComp(
return result, nil
}
} else {
switch colType {
case "DECIMAL":
switch {
case colType == "DECIMAL":
// For decimals, remove any trailing zeroes.
row1[j] = trimDecimalTrailingZeroes(row1[j])
row2[j] = trimDecimalTrailingZeroes(row2[j])
case "[]DECIMAL":
case isDecimalArray(colType):
// For decimal arrays, remove any trailing zeroes from each
// decimal.
row1[j] = trimDecimalsTrailingZeroesInArray(row1[j])
Expand Down
8 changes: 8 additions & 0 deletions pkg/cmd/roachtest/tests/query_comparison_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,14 @@ func TestUnsortedMatricesDiff(t *testing.T) {
exactMatch: false,
approxMatch: true,
},
{
name: "multi row 0 in array matches -0 in array, lib/pq type name",
colTypes: []string{"_FLOAT4"}, // this is how []FLOAT4 is named in lib/pq driver
t1: [][]string{{"NULL"}, {"{-1}"}, {"{-0}"}, {"{0}"}, {"{NaN}"}},
t2: [][]string{{"NULL"}, {"{-1}"}, {"{0}"}, {"{0}"}, {"{NaN}"}},
exactMatch: false,
approxMatch: true,
},
}
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
Expand Down

0 comments on commit d0be0aa

Please sign in to comment.