inspect: fix false row count mismatch errors after IMPORT by spilchen · Pull Request #168158 · cockroachdb/cockroach

spilchen · 2026-04-10T18:14:12Z

IMPORT validates data integrity by running an inspect job that checks the row count against the expected number of imported rows. When the primary index has an empty span (a range with no rows), the inspect job incorrectly inflates the accumulated row count, producing a spurious RowCountMismatch error. This makes it appear that the imported data is corrupt when the data is actually correct.

The root cause is in getPredicateAndQueryArgs, which returned an empty predicate for empty spans. Downstream queries then ran without a WHERE clause, counting all rows in the table instead of zero for the empty span. The fix derives query bounds directly from the span boundaries when no rows exist, keeping all queries properly scoped. The function now also returns a hasRows bool so callers can distinguish empty spans without relying on a sentinel empty string.

Closes #168001

Release note: None

trunk-io · 2026-04-10T18:14:17Z

😎 Merged successfully - details.

cockroach-teamcity · 2026-04-10T18:14:24Z

This change is

fqazi

Nice find @spilchen

@fqazi reviewed 8 files and all commit messages, and made 1 comment.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on spilchen).

bghal

Stamping to unblock the release but had some questions on the interface.

bghal · 2026-04-10T18:42:21Z

-	// If no rows exist in the primary index span, we still need to check for dangling
-	// secondary index entries. We run the check with an empty predicate, which will
-	// scan the entire secondary index within the span. Any secondary index entries found
-	// will be dangling since there are no corresponding primary index rows.


super nit: I feel like some detail is lost here.

bghal · 2026-04-10T18:59:54Z

 			log.Dev.Infof(ctx, "skipping hash precheck for index %s: column type not compatible with datums_to_bytes",
 				c.secIndex.GetName())
 		} else {
 			match, rowCount, hashErr := c.hashesMatch(ctx, allColNames, predicate, queryArgs)


Is the overly broad count here? Would it make sense for the inspectCheckRowCount interface to expose an invalid count rather than relying on its user doing the its own scan validation?

I can make the interface changes in a subsequent diff if they're sensible and not worth holding off on.

Yes, this is where we double counted. The query had no predicates so scanned the entire table.

I'm not sure 100% sure what you are suggesting for the interface change. Can you clarify? It sounds like something to handle in a follow-on as it doesn't impact the fix.

I'm thinking that c.rowCount should never be filled if we know it's a bad count from hasRows.

Noted down my thoughts but best as a follow-up.

spilchen

Thanks for the reviews

@spilchen made 4 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on bghal and fqazi).

spilchen · 2026-04-10T20:09:00Z

-	// If no rows exist in the primary index span, we still need to check for dangling
-	// secondary index entries. We run the check with an empty predicate, which will
-	// scan the entire secondary index within the span. Any secondary index entries found
-	// will be dangling since there are no corresponding primary index rows.


spilchen · 2026-04-10T20:09:01Z

 			log.Dev.Infof(ctx, "skipping hash precheck for index %s: column type not compatible with datums_to_bytes",
 				c.secIndex.GetName())
 		} else {
 			match, rowCount, hashErr := c.hashesMatch(ctx, allColNames, predicate, queryArgs)


Yes, this is where we double counted. The query had no predicates so scanned the entire table.

I'm not sure 100% sure what you are suggesting for the interface change. Can you clarify? It sounds like something to handle in a follow-on as it doesn't impact the fix.

IMPORT validates data integrity by running an inspect job that checks the row count against the expected number of imported rows. When the primary index has an empty span (a range with no rows), the inspect job incorrectly inflates the accumulated row count, producing a spurious RowCountMismatch error. This makes it appear that the imported data is corrupt when the data is actually correct. The root cause is in getPredicateAndQueryArgs, which returned an empty predicate for empty spans. Downstream queries then ran without a WHERE clause, counting all rows in the table instead of zero for the empty span. The fix derives query bounds directly from the span boundaries when no rows exist, keeping all queries properly scoped. The function now also returns a hasRows bool so callers can distinguish empty spans without relying on a sentinel empty string. Closes cockroachdb#168001 Release note: None Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

bghal · 2026-04-10T20:17:45Z

 // its primary validation.
 type inspectCheckRowCount interface {
 	// Rows returns the number of rows counted by the check.
 	RowCount() uint64


Suggested change

RowCount() *uint64

The interface can indicate there is no valid row count for the span.

bghal · 2026-04-10T20:24:09Z

 		if check, ok := check.(inspectCheckRowCount); ok {
 			data.SpanRowCount = check.RowCount()
 			return nil
 		}


Suggested change

if check, ok := check.(inspectCheckRowCount); ok {

if rowCount := check.RowCount(); rowCount != nil {

data.SpanRowCount = check.RowCount()

return nil

}

}

Same behavior but the row count check never gets an incorrect value from the consistency check.

bghal · 2026-04-10T20:25:22Z

 			log.Dev.Infof(ctx, "skipping hash precheck for index %s: column type not compatible with datums_to_bytes",
 				c.secIndex.GetName())
 		} else {
 			match, rowCount, hashErr := c.hashesMatch(ctx, allColNames, predicate, queryArgs)


I'm thinking that c.rowCount should never be filled if we know it's a bad count from hasRows.

Noted down my thoughts but best as a follow-up.

spilchen · 2026-04-10T22:51:41Z

TFTR!

/trunk merge

rafiss · 2026-04-10T23:33:57Z

blathers backport 26.2.0-rc

blathers-crl · 2026-04-10T23:34:03Z

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.

Issue #168001: branch-release-26.2.0-rc.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

IMPORT row count validation has been failing on drt-chaos-aws with multiple distinct failure modes (off-by-one, 2x doubling, deficit) despite prior fixes (cockroachdb#165697, cockroachdb#168158). This is suspected to be an accounting problem and not actual data corruption. Change the default value of `bulkio.import.row_count_validation.mode` from `async` to `off` to prevent spurious INSPECT errors from surfacing in customer environments. The setting remains user-configurable and can be re-enabled once the root cause of cockroachdb#168396 is identified and fixed. Metamorphic test builds continue to randomly exercise all three modes (off, async, sync). Fixes: cockroachdb#168400 Epic: none Release note: none Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

spilchen self-assigned this Apr 10, 2026

spilchen force-pushed the gh-168001/260410/1511/inspect/fix-row-count branch from 7a0be44 to 4ef9b62 Compare April 10, 2026 18:30

spilchen marked this pull request as ready for review April 10, 2026 18:31

spilchen requested a review from a team as a code owner April 10, 2026 18:31

fqazi approved these changes Apr 10, 2026

View reviewed changes

bghal approved these changes Apr 10, 2026

View reviewed changes

cockroach-teamcity added the X-perf-gain Microbenchmarks CI: Added if a performance gain is detected label Apr 10, 2026

spilchen force-pushed the gh-168001/260410/1511/inspect/fix-row-count branch from 4ef9b62 to 86bc0d2 Compare April 10, 2026 20:00

spilchen commented Apr 10, 2026

View reviewed changes

spilchen force-pushed the gh-168001/260410/1511/inspect/fix-row-count branch from 86bc0d2 to d558c05 Compare April 10, 2026 20:11

bghal reviewed Apr 10, 2026

View reviewed changes

spilchen added the backport-26.2.x Flags PRs that need to be backported to 26.2 label Apr 10, 2026

spilchen mentioned this pull request Apr 10, 2026

inspect: change inspectCheckRowCount.RowCount to return *uint64 for empty-span safety #168184

Open

trunk-io bot merged commit aa58e85 into cockroachdb:master Apr 10, 2026
30 checks passed

celeste-cockroachdb bot added the target-release-26.3.0 label Apr 10, 2026

spilchen mentioned this pull request Apr 10, 2026

release-26.2: inspect: fix false row count mismatch errors after IMPORT #168185

Merged

blathers-crl bot mentioned this pull request Apr 10, 2026

sql: import validation job fails with 2x as many rows #168001

Closed

spilchen mentioned this pull request Apr 10, 2026

release-26.2.0-rc: inspect: fix false row count mismatch errors after IMPORT #168186

Merged

This was referenced Apr 15, 2026

sql: import validation jobs report 2x expected row count on drt-chaos-aws #165540

Closed

sql/importer: IMPORT row count validation still failing after fixes on drt-chaos-aws #168396

Open

spilchen mentioned this pull request Apr 15, 2026

sql/importer: disable INSPECT after IMPORT validation #168400

Open

spilchen mentioned this pull request Apr 15, 2026

release-26.2: sql/importer: disable INSPECT after IMPORT validation by default #168403

Merged

spilchen mentioned this pull request Apr 15, 2026

release-26.2.0-rc: release-26.2: sql/importer: disable INSPECT after IMPORT validation by default #168424

Merged

Conversation

spilchen commented Apr 10, 2026

Uh oh!

trunk-io bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Apr 10, 2026

Uh oh!

fqazi left a comment

Choose a reason for hiding this comment

Uh oh!

bghal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bghal Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spilchen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spilchen commented Apr 10, 2026

Uh oh!

Uh oh!

rafiss commented Apr 10, 2026

Uh oh!

blathers-crl bot commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

trunk-io bot commented Apr 10, 2026 •

edited

Loading

bghal Apr 10, 2026 •

edited

Loading