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

sql/parser: parse FOR UPDATE lock wait policies #43868

Merged

Conversation

nvanbenschoten
Copy link
Member

This commit adds parsing-only support for the SKIP LOCKED and NOWAIT
lock wait policies that accompany the FOR UPDATE locking strength
specifiers. These are still rejected with unimplemented errors, but
they now at least make it to the optimizer, where we can make a
better decision about what to do with them.

Release note: None

@nvanbenschoten nvanbenschoten requested a review from knz January 10, 2020 03:53
@nvanbenschoten nvanbenschoten requested a review from a team as a code owner January 10, 2020 03:53
@cockroach-teamcity
Copy link
Member

This change is Reviewable

This commit adds parsing-only support for the SKIP LOCKED and NOWAIT
lock wait policies that accompany the FOR UPDATE locking strength
specifiers. These are still rejected with unimplemented errors, but
they now at least make it to the optimizer, where we can make a
better decision about what to do with them.

Release note: None
@nvanbenschoten nvanbenschoten force-pushed the nvanbenschoten/lockWaitPolicy branch from 776d308 to 04aaabc Compare January 10, 2020 10:31
Copy link
Contributor

@knz knz 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 7 of 7 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@nvanbenschoten
Copy link
Member Author

bors r+

craig bot pushed a commit that referenced this pull request Jan 10, 2020
43868: sql/parser: parse FOR UPDATE lock wait policies r=nvanbenschoten a=nvanbenschoten

This commit adds parsing-only support for the SKIP LOCKED and NOWAIT
lock wait policies that accompany the FOR UPDATE locking strength
specifiers. These are still rejected with unimplemented errors, but
they now at least make it to the optimizer, where we can make a
better decision about what to do with them.

Release note: None

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
@craig
Copy link
Contributor

craig bot commented Jan 10, 2020

Build succeeded

@craig craig bot merged commit 04aaabc into cockroachdb:master Jan 10, 2020
@nvanbenschoten nvanbenschoten deleted the nvanbenschoten/lockWaitPolicy branch January 14, 2020 22:56
Copy link
Collaborator

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

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

Do you imagine that we would ever implement SKIP LOCKED, despite it dangerously providing an inconsistent view of the data?

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

Copy link
Member Author

@nvanbenschoten nvanbenschoten left a comment

Choose a reason for hiding this comment

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

I doubt we will want to implement it unless we find serious customer desire for the feature. NO WAIT seems like a more reasonable alternate wait policy, so I can imagine us implementing that at some point.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

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.

4 participants