Skip to content

PostgreSQL: Support FOR NO KEY UPDATE and FOR KEY SHARE - #2788

Open
abhinav-phi wants to merge 1 commit into
apache:mainfrom
abhinav-phi:fix/2778-row-locking-clauses
Open

abhinav-phi wants to merge 1 commit into
apache:mainfrom
abhinav-phi:fix/2778-row-locking-clauses

Conversation

@abhinav-phi

Copy link
Copy Markdown

FOR NO KEY UPDATE and FOR KEY SHARE are valid PostgreSQL row locking strengths but were rejected:

sql parser error: Expected: one of UPDATE or SHARE, found: NO
sql parser error: Expected: one of UPDATE or SHARE, found: KEY

Add LockType::NoKeyUpdate / LockType::KeyShare, display them as NO KEY UPDATE / KEY SHARE, and parse them in parse_lock before the existing OF / NOWAIT / SKIP LOCKED handling.

Grammar: https://www.postgresql.org/docs/current/sql-select.html#SQL-FOR-UPDATE-SHARE

Tests cover both forms with OF and wait policies in PostgreSQL and Generic, and incomplete forms are rejected. Added fuzz seed fuzz/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.

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 slopgobblercobbler left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Audited the PostgreSQL row-locking strength extension in src/ast/query.rs and src/parser/mod.rs:

  • AST representation and Display roundtrip:
    • Adding LockType::NoKeyUpdate and LockType::KeyShare accurately models PostgreSQL's row-level locking strengths (FOR NO KEY UPDATE and FOR KEY SHARE).
    • fmt::Display outputs "NO KEY UPDATE" and "KEY SHARE", integrating seamlessly with LockClause formatting (write!(f, "FOR {}", self.lock_type)) to achieve byte-exact roundtrip serialization.
  • Parser lookahead and error reporting:
    • Lookahead sequencing in parse_lock checks NO -> [KEY, UPDATE] and KEY -> SHARE before falling back to [UPDATE, SHARE]. This prevents ambiguous keyword matching while providing precise diagnostics on truncated or mismatched clauses (e.g. FOR NO KEY SHARE rejecting on SHARE rather than failing generically).
    • Trailing clauses (OF, NOWAIT, SKIP LOCKED) execute after lock_type resolution, correctly supporting compositions such as FOR NO KEY UPDATE OF t SKIP LOCKED and FOR KEY SHARE OF t NOWAIT.
  • 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.sql ensures ongoing fuzzer validation.

Clean, idiomatic Rust AST contribution that closes a noticeable Postgres syntax gap with zero breaking changes.

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.

PostgreSQL: support FOR NO KEY UPDATE and FOR KEY SHARE row-locking clauses

2 participants