Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

db: fix overlap check for flushable ingest excises #3968

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

itsbilal
Copy link
Member

Previously, we would incorrectly return false (denoting no overlap) between passed-in UserKeyBounds and a flushable ingest, if we see a file that starts after the passed-in bounds, even if there was an excise span overlapping with those bounds. This change fixes that, and adds a unit test to catch this edge case.

Fixes #3963.

Previously, we would incorrectly return false (denoting no overlap)
between passed-in UserKeyBounds and a flushable ingest, if we
see a file that starts after the passed-in bounds, even if there
was an excise span overlapping with those bounds. This change
fixes that, and adds a unit test to catch this edge case.

Fixes cockroachdb#3963.
@itsbilal itsbilal requested a review from jbowens September 26, 2024 02:48
@itsbilal itsbilal requested a review from a team as a code owner September 26, 2024 02:48
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 3 files at r1, all commit messages.
Reviewable status: 1 of 3 files reviewed, all discussions resolved (waiting on @RaduBerinde)

@itsbilal
Copy link
Member Author

TFTR!

@itsbilal itsbilal merged commit be56747 into cockroachdb:master Sep 26, 2024
12 checks passed
Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for fixing so quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

internal/metamorphic: TestMeta failed [spurious key]
4 participants