From d5d40099c0b5bee2df0f2f4303b797089347c0a8 Mon Sep 17 00:00:00 2001
From: Lucy Zhang <lucy-zhang@users.noreply.github.com>
Date: Thu, 6 Dec 2018 13:31:09 -0500
Subject: [PATCH] sql: fix SCRUB index key order checking

Previously `SCRUB` would erroneously report that index keys were out of order
for columns other than the first column in an index. This fixes the bug.

Fixes #32874.

Release note (bug fix): Fixed bug where SCRUB would erroneously report that
index keys were out of order.
---
 pkg/sql/logictest/testdata/logic_test/scrub | 11 +++++++++++
 pkg/sql/row/fetcher.go                      | 19 ++++++++++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/pkg/sql/logictest/testdata/logic_test/scrub b/pkg/sql/logictest/testdata/logic_test/scrub
index f8a5fb903194..660f107e882e 100644
--- a/pkg/sql/logictest/testdata/logic_test/scrub
+++ b/pkg/sql/logictest/testdata/logic_test/scrub
@@ -136,3 +136,14 @@ EXPERIMENTAL SCRUB DATABASE seq_db
 
 statement error pq: "seq_db.public.my_seq" is not a table
 EXPERIMENTAL SCRUB TABLE seq_db.my_seq
+
+# Test for false positives when checking key order (#32874)
+
+statement ok
+CREATE TABLE test.order (a INT, b INT, c INT, CONSTRAINT "primary" PRIMARY KEY (a, b, c DESC))
+
+statement ok
+INSERT INTO test.order VALUES (0, 0, 0), (0, 0, 1), (0, 1, 0), (0, 1, 1), (1, 0, 0);
+
+query TTTTTTTT
+EXPERIMENTAL SCRUB TABLE test.order WITH OPTIONS PHYSICAL
diff --git a/pkg/sql/row/fetcher.go b/pkg/sql/row/fetcher.go
index 1d9d401181d2..4dbc5fdebdc9 100644
--- a/pkg/sql/row/fetcher.go
+++ b/pkg/sql/row/fetcher.go
@@ -1252,6 +1252,10 @@ func (rf *Fetcher) checkKeyOrdering(ctx context.Context) error {
 	}
 
 	evalCtx := tree.EvalContext{}
+	// Iterate through columns in order, comparing each value to the value in the
+	// previous row in that column. When the first column with a differing value
+	// is found, compare the values to ensure the ordering matches the column
+	// ordering.
 	for i, id := range rf.rowReadyTable.index.ColumnIDs {
 		idx := rf.rowReadyTable.colIdxMap[id]
 		result := rf.rowReadyTable.decodedRow[idx].Compare(&evalCtx, rf.rowReadyTable.lastDatums[idx])
@@ -1262,11 +1266,16 @@ func (rf *Fetcher) checkKeyOrdering(ctx context.Context) error {
 			expectedDirection = sqlbase.IndexDescriptor_ASC
 		}
 
-		if expectedDirection == sqlbase.IndexDescriptor_ASC && result < 0 ||
-			expectedDirection == sqlbase.IndexDescriptor_DESC && result > 0 {
-			return scrub.WrapError(scrub.IndexKeyDecodingError,
-				errors.Errorf("key ordering did not match datum ordering. IndexDescriptor=%s",
-					expectedDirection))
+		if result != 0 {
+			if expectedDirection == sqlbase.IndexDescriptor_ASC && result < 0 ||
+				expectedDirection == sqlbase.IndexDescriptor_DESC && result > 0 {
+				return scrub.WrapError(scrub.IndexKeyDecodingError,
+					errors.Errorf("key ordering did not match datum ordering. IndexDescriptor=%s",
+						expectedDirection))
+			}
+			// After the first column with a differing value is found, the remaining
+			// columns are skipped (see #32874).
+			break
 		}
 	}
 	return nil