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

storage: add nextKeyIgnoringTime() to MVCCIncrementalIterator #83729

Merged
merged 1 commit into from
Jul 8, 2022

Conversation

msbutler
Copy link
Collaborator

@msbutler msbutler commented Jul 1, 2022

The MVCCInvermentalIterator's NextKeyIgnoringTime() function was previously
deleted in #82691, as there wasn't any use for it at the time. Now, the new
Delete Range with predicate filtering will need it (#83676).

This PR also cleans up duplicate code used to test NextIgnoringTime and
NextKeyIgnoringTime.

Release note: None

@msbutler msbutler requested a review from erikgrinaker July 1, 2022 21:16
@msbutler msbutler self-assigned this Jul 1, 2022
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@msbutler msbutler removed the request for review from erikgrinaker July 1, 2022 21:49
@msbutler msbutler force-pushed the butler-nextkeyignoringtime branch from 0b69f89 to 3d4600d Compare July 6, 2022 15:30
@msbutler msbutler marked this pull request as ready for review July 6, 2022 15:31
@msbutler msbutler requested a review from a team as a code owner July 6, 2022 15:31
Copy link
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

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

Can you add some more test cases to testdata/mvcc_histories/range_key_iter_incremental, with support for NextKeyIgnoringTime() in mvcc_histories_test.go?

Also, please update the PR/commit messages with the correct method name.

Otherwise looks good!

@msbutler msbutler force-pushed the butler-nextkeyignoringtime branch from 24003fa to 6a93f0c Compare July 7, 2022 22:02
@msbutler msbutler changed the title storage: add nextKeyIgnoreTime() to MVCCIncrementalIterator storage: add nextKeyIgnoringTime() to MVCCIncrementalIterator Jul 7, 2022
Copy link
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

The MVCCInvermentalIterator's NextKeyIgnoringTime() function was previously
deleted in cockroachdb#82691, as there wasn't any use for it at the time. Now, the new
Delete Range with predicate filtering will need it (cockroachdb#83676).

This PR also cleans up duplicate code used to test NextIgnoringTime and
NextKeyIgnoringTime.

Release note: None
@msbutler msbutler force-pushed the butler-nextkeyignoringtime branch from 6a93f0c to 8c9df8a Compare July 8, 2022 00:00
@msbutler
Copy link
Collaborator Author

msbutler commented Jul 8, 2022

TFTR!!

bors r=erikgrinaker

@craig
Copy link
Contributor

craig bot commented Jul 8, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Jul 8, 2022

Build succeeded:

@craig craig bot merged commit 672f201 into cockroachdb:master Jul 8, 2022
@msbutler msbutler deleted the butler-nextkeyignoringtime branch July 8, 2022 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants