Merge #42292

42292: workload/tpch: fix the check of the output on TPCH workload r=yuzefovich a=yuzefovich

Previously, when doing the check that TPCH queries return the expected
output, we would round down both the expected and the actual float
values to a hundredth, and then we would confirm that the rounded values
differ by no more than 0.01 (this is what the spec requires). However,
this can be problematic in some scenarios. It is possible, for example,
for two values 0.601 and 0.609 be rounded in such a way that they are
represented by 0.599998 and 0.610001, respectively. Then, when the check
that the difference is no greater than 0.01 would fail although the
numbers are correct (according to the spec). Now this is fixed by
rounding to a thousandth.

It turns out that this was the problem that I occasionally observed when
running TPCHVec roachtest - it would show up as "exit status 1" because
of the failed check.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

pkg/cmd/roachtest/tpchvec.go

@@ -78,11 +78,6 @@ RESTORE tpch.* FROM 'gs://cockroach-fixtures/workload/tpch/scalefactor=1/backup'
 		t.Status("waiting for full replication")
 		waitForFullReplication(t, conn)
 		timeByQueryNum := make([]map[int][]float64, 2)
-		// Note that the order in which we run the configuration is important:
-		// there are some issues with dropping a view (created in query 15), so if
-		// we run vec off first, the vec on config run might error out.
-		// TODO(yuzefovich): figure out what is the root problem or create an issue
-		// about it.
 		for configIdx, vectorize := range []bool{true, false} {
 			// To reduce the variance on the first query we're interested in, we'll
 			// do an aggregation over all tables. This will make comparison on two
@@ -122,6 +117,8 @@ RESTORE tpch.* FROM 'gs://cockroach-fixtures/workload/tpch/scalefactor=1/backup'
 				queriesToRun, vectorizeSetting, nodeCount)
 			workloadOutput, err := repeatRunWithBuffer(ctx, c, t.l, firstNode, operation, cmd)
 			if err != nil {
+				// Note: if you see an error like "exit status 1", it is likely caused
+				// by the erroneous output of the query.
 				t.Fatal(err)
 			}
 			t.l.Printf(string(workloadOutput))

pkg/workload/tpch/tpch.go

@@ -242,14 +242,20 @@ func (w *worker) run(ctx context.Context) error {
 									queryName, err, numRows, i, actualValue, expectedValue)
 							}
 							// TPC-H spec requires 0.01 precision for DECIMALs, so we will
-							// first round the values to use in the comparison.
-							expectedFloatRounded, err := strconv.ParseFloat(fmt.Sprintf("%.2f", expectedFloat), 64)
+							// first round the values to use in the comparison. Note that we
+							// round to a thousandth so that values like 0.601 and 0.609 were
+							// always considered to differ by less than 0.01 (due to the
+							// nature of representation of floats, it is possible that those
+							// two values when rounded to a hundredth would be represented as
+							// something like 0.59999 and 0.610001 which differ by more than
+							// 0.01).
+							expectedFloatRounded, err := strconv.ParseFloat(fmt.Sprintf("%.3f", expectedFloat), 64)
 							if err != nil {
 								return errors.Errorf("[q%s] failed parsing rounded expected value as float64 with %s\n"+
 									"wrong result in row %d in column %d: got %q, expected %q",
 									queryName, err, numRows, i, actualValue, expectedValue)
 							}
-							actualFloatRounded, err := strconv.ParseFloat(fmt.Sprintf("%.2f", actualFloat), 64)
+							actualFloatRounded, err := strconv.ParseFloat(fmt.Sprintf("%.3f", actualFloat), 64)
 							if err != nil {
 								return errors.Errorf("[q%s] failed parsing rounded actual value as float64 with %s\n"+
 									"wrong result in row %d in column %d: got %q, expected %q",