Skip to content

Commit

Permalink
Merge #32908
Browse files Browse the repository at this point in the history
32908: sql: fix SCRUB index key order checking r=lucy-zhang a=lucy-zhang

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.

Co-authored-by: Lucy Zhang <lucy-zhang@users.noreply.github.com>
  • Loading branch information
craig[bot] and lucy-zhang committed Dec 6, 2018
2 parents 4367a43 + d5d4009 commit 11405f2
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
11 changes: 11 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/scrub
Original file line number Diff line number Diff line change
Expand Up @@ -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
19 changes: 14 additions & 5 deletions pkg/sql/row/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -1200,6 +1200,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])
Expand All @@ -1210,11 +1214,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
Expand Down

0 comments on commit 11405f2

Please sign in to comment.