Skip to content

Commit 92e2814

Browse files
committed
sql: add fetch row count assertions in row-by-row lookup joiner
Assertions have been added to the join reader that ensure that a lookup join on key columns fetches the expected number of rows. Fixes #135696 Release note: None
1 parent dd052f9 commit 92e2814

File tree

1 file changed

+29
-26
lines changed

1 file changed

+29
-26
lines changed

pkg/sql/rowexec/joinreader.go

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,10 @@ type joinReader struct {
209209
// curBatchInputRowCount is the number of input rows in the current batch.
210210
curBatchInputRowCount int64
211211

212+
// If set, the lookup columns form a key in the target table and thus each
213+
// lookup has at most one result.
214+
lookupColumnsAreKey bool
215+
212216
// lockingWaitPolicy is the wait policy for the underlying rowFetcher.
213217
lockingWaitPolicy descpb.ScanLockingWaitPolicy
214218

@@ -380,6 +384,7 @@ func newJoinReader(
380384
outputGroupContinuationForLeftRow: spec.OutputGroupContinuationForLeftRow,
381385
parallelize: parallelize,
382386
readerType: readerType,
387+
lookupColumnsAreKey: spec.LookupColumnsAreKey,
383388
lockingWaitPolicy: spec.LockingWaitPolicy,
384389
txn: txn,
385390
usesStreamer: useStreamer,
@@ -934,9 +939,8 @@ func (jr *joinReader) readInput() (
934939
jr.resetScratchWhenReadingInput = false
935940
}
936941

937-
// Assert that the correct number of rows were fetched in the last batch,
938-
// for index joins.
939-
if err := jr.assertIndexJoinRowCounts(); err != nil {
942+
// Assert that the correct number of rows were fetched in the last batch.
943+
if err := jr.assertBatchRowCounts(); err != nil {
940944
jr.MoveToDraining(err)
941945
return jrStateUnknown, nil, jr.DrainHelper()
942946
}
@@ -1114,30 +1118,29 @@ func (jr *joinReader) readInput() (
11141118
return jrFetchingLookupRows, outRow, nil
11151119
}
11161120

1117-
// assertIndexJoinRowCounts performs assertions to prevent silently returning
1118-
// incorrect results, e.g., if an index is corrupt. In the common case, the
1119-
// number of fetched rows in an index join should be equal to the number of
1120-
// input rows. The only exception is when the locking wait policy is
1121-
// SKIP LOCKED, in which case the number of fetched rows may be less than
1122-
// the number of input rows, but never greater.
1123-
func (jr *joinReader) assertIndexJoinRowCounts() error {
1124-
if jr.readerType != indexJoinReaderType {
1125-
return nil
1121+
// assertBatchRowCounts performs assertions to prevent silently returning
1122+
// incorrect results, e.g., if the lookup index is corrupt.
1123+
func (jr *joinReader) assertBatchRowCounts() error {
1124+
// An index join without SKIP LOCKED should fetch exactly one row for each
1125+
// input row.
1126+
nonSkippingIndexJoin := jr.readerType == indexJoinReaderType &&
1127+
jr.lockingWaitPolicy != descpb.ScanLockingWaitPolicy_SKIP_LOCKED
1128+
if nonSkippingIndexJoin && jr.curBatchRowsRead != jr.curBatchInputRowCount {
1129+
return errors.AssertionFailedf(
1130+
"expected to fetch %d rows, found %d",
1131+
jr.curBatchInputRowCount, jr.curBatchRowsRead,
1132+
)
11261133
}
1127-
if jr.lockingWaitPolicy == descpb.ScanLockingWaitPolicy_SKIP_LOCKED {
1128-
if jr.curBatchRowsRead > jr.curBatchInputRowCount {
1129-
return errors.AssertionFailedf(
1130-
"expected to fetch no more than %d rows, found %d",
1131-
jr.curBatchInputRowCount, jr.curBatchRowsRead,
1132-
)
1133-
}
1134-
} else {
1135-
if jr.curBatchRowsRead != jr.curBatchInputRowCount {
1136-
return errors.AssertionFailedf(
1137-
"expected to fetch %d rows, found %d",
1138-
jr.curBatchInputRowCount, jr.curBatchRowsRead,
1139-
)
1140-
}
1134+
// An index join with SKIP LOCKED or a lookup join where the lookup columns
1135+
// form a key should fetch at most one row for each input row.
1136+
skippingIndexJoin := jr.readerType == indexJoinReaderType &&
1137+
jr.lockingWaitPolicy == descpb.ScanLockingWaitPolicy_SKIP_LOCKED
1138+
if (skippingIndexJoin || jr.lookupColumnsAreKey) &&
1139+
jr.curBatchRowsRead > jr.curBatchInputRowCount {
1140+
return errors.AssertionFailedf(
1141+
"expected to fetch no more than %d rows, found %d",
1142+
jr.curBatchInputRowCount, jr.curBatchRowsRead,
1143+
)
11411144
}
11421145
return nil
11431146
}

0 commit comments

Comments
 (0)