-
Notifications
You must be signed in to change notification settings - Fork 3.9k
sql: add row lock modes; make FOR UPDATE a no-op #40206
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but should we go ahead and make all of these no-ops instead of just FOR UPDATE? I think the same logic applies to all of these (except for the fact that FOR UPDATE appears to be far more commonly used than the others).
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @andy-kimball, @bdarnell, and @rytaft)
We can do that - I hadn't thought about the semantics of all of the other ones, which also seem subtle - but I think you're right that none of them are observably different when all transactions are serializable. |
This commit adds parsing for the row locking modes: FOR UPDATE, FOR NO KEY UPDATE, FOR SHARE, FOR KEY UPDATE. All return unimplemented errors, except for FOR UPDATE, which behaves as a no-op. Release note (sql change): support parsing the FOR UPDATE modifier on SELECT clauses, treating it as a no-op, since CockroachDB's transactions only operate in SERIALIZABLE mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 9 of 9 files at r1, 4 of 4 files at r2.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @andy-kimball)
bors r+ |
40206: sql: add row lock modes; make FOR UPDATE a no-op r=jordanlewis a=jordanlewis This commit adds parsing for the row locking modes: FOR UPDATE, FOR NO KEY UPDATE, FOR SHARE, FOR KEY UPDATE. All return unimplemented errors, except for FOR UPDATE, which behaves as a no-op. Closes #6583. Added #40205 to track the remaining kinds of row locking. Release note (sql change): support parsing the FOR UPDATE modifier on SELECT clauses, treating it as a no-op, since CockroachDB's transactions only operate in SERIALIZABLE mode. Co-authored-by: Jordan Lewis <[email protected]>
Build succeeded |
This commit adds parsing for the row locking modes: FOR UPDATE, FOR NO
KEY UPDATE, FOR SHARE, FOR KEY UPDATE.
All return unimplemented errors, except for FOR UPDATE, which behaves as
a no-op.
Closes #6583.
Added #40205 to track the remaining kinds of row locking.
Release note (sql change): support parsing the FOR UPDATE modifier on
SELECT clauses, treating it as a no-op, since CockroachDB's transactions
only operate in SERIALIZABLE mode.