Repository navigation
PostgreSQL: Support FOR NO KEY UPDATE and FOR KEY SHARE - #2788
Open
abhinav-phi wants to merge 1 commit into
Open
abhinav-phi wants to merge 1 commit into
abhinav-phi wants to merge 1 commit into
Conversation
Add LockType::NoKeyUpdate and LockType::KeyShare, parse both row-locking strengths in parse_lock, and cover them with PostgreSQL/Generic round-trip tests, error-path tests, and a fuzz seed. Refs apache#2778
slopgobblercobbler
left a comment
There was a problem hiding this comment.
Audited the PostgreSQL row-locking strength extension in src/ast/query.rs and src/parser/mod.rs:
- AST representation and Display roundtrip:
- Adding
LockType::NoKeyUpdateandLockType::KeyShareaccurately models PostgreSQL's row-level locking strengths (FOR NO KEY UPDATEandFOR KEY SHARE). fmt::Displayoutputs"NO KEY UPDATE"and"KEY SHARE", integrating seamlessly withLockClauseformatting (write!(f, "FOR {}", self.lock_type)) to achieve byte-exact roundtrip serialization.
- Adding
- Parser lookahead and error reporting:
- Lookahead sequencing in
parse_lockchecksNO->[KEY, UPDATE]andKEY->SHAREbefore falling back to[UPDATE, SHARE]. This prevents ambiguous keyword matching while providing precise diagnostics on truncated or mismatched clauses (e.g.FOR NO KEY SHARErejecting onSHARErather than failing generically). - Trailing clauses (
OF,NOWAIT,SKIP LOCKED) execute afterlock_typeresolution, correctly supporting compositions such asFOR NO KEY UPDATE OF t SKIP LOCKEDandFOR KEY SHARE OF t NOWAIT.
- Lookahead sequencing in
- Dialect behavior:
- Both PostgreSQL and Generic dialects correctly accept the new clauses without altering syntax rules for other database engines.
- Test and fuzz coverage:
- Unit tests comprehensively verify roundtrip formatting, AST structure (
lock.of,lock.nonblock), and negative test cases for incomplete or invalid token sequences. - Adding
fuzz/fuzz_seeds/postgres_2778_row_locking.sqlensures ongoing fuzzer validation.
- Unit tests comprehensively verify roundtrip formatting, AST structure (
Clean, idiomatic Rust AST contribution that closes a noticeable Postgres syntax gap with zero breaking changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FOR NO KEY UPDATEandFOR KEY SHAREare valid PostgreSQL row locking strengths but were rejected:Add
LockType::NoKeyUpdate/LockType::KeyShare, display them asNO KEY UPDATE/KEY SHARE, and parse them inparse_lockbefore the existingOF/NOWAIT/SKIP LOCKEDhandling.Grammar: https://www.postgresql.org/docs/current/sql-select.html#SQL-FOR-UPDATE-SHARE
Tests cover both forms with
OFand wait policies in PostgreSQL and Generic, and incomplete forms are rejected. Added fuzz seedfuzz/fuzz_seeds/postgres_2778_row_locking.sql.Validation:
cargo test --all-features,cargo fmt --all -- --check,cargo clippy --all-targets --all-features -- -D warnings.Fixes #2778.
AI-ASSISTED: yes. human reviewer: @abhinav-phi.