Skip to content

fix(plpgsql): sync generator with committed grammar#30

Open
AlexS778 wants to merge 1 commit into
gmr:mainfrom
AlexS778:fix/generator-drift
Open

fix(plpgsql): sync generator with committed grammar#30
AlexS778 wants to merge 1 commit into
gmr:mainfrom
AlexS778:fix/generator-drift

Conversation

@AlexS778

@AlexS778 AlexS778 commented May 21, 2026

Copy link
Copy Markdown
Contributor

Update the PL/pgSQL grammar generator so it matches the committed plpgsql/grammar.js for drift-sensitive rules.

  • emit alias declarations before variable declarations
  • emit FETCH as optional FROM plus required cursor name
  • add a lightweight generator drift test for decl_statement and stmt_fetch

This prevents future regeneration from undoing the current committed grammar shape.

Summary

This PR fixes drift between the committed plpgsql/grammar.js and its generator template in script/generate-plpgsql-grammar.js.

The committed grammar already has the intended behavior. This PR moves that behavior back into the generator so future regeneration does not undo it.

Changes

Declaration ordering

Updated decl_statement generation so alias declarations are emitted before variable declarations.

This preserves the existing intended behavior where:

x ALIAS FOR old_name;

is parsed as an alias declaration rather than being confused with a variable declaration whose type name starts with ALIAS.

FETCH statement shape
Updated generated stmt_fetch from:

optional(seq($.kw_from, $.any_identifier)),
$.kw_into,

to:

optional($.kw_from),
$.any_identifier,
$.kw_into,

This matches the committed grammar and supports both:

FETCH curs INTO x;
FETCH NEXT FROM curs INTO x;

Testing

Ran lightweight generator drift tests:

node --test script/*.test.js

Result:

4/4 passed

Ran PL/pgSQL corpus tests:

cd plpgsql
../node_modules/.bin/tree-sitter test

Result:

60/60 passed

Full PostgreSQL parser regeneration was not run because this PR only updates the PL/pgSQL generator template and does not change the committed generated PL/pgSQL grammar.

It does not change the broader PL/pgSQL scanner architecture or the _sql_body handling discussed separately.

Closes #29

Summary by CodeRabbit

  • Tests

    • Added test to detect drift between generated and committed grammar for PL/pgSQL parsing rules.
  • Chores

    • Updated internal grammar generation script with adjusted rule ordering and token parsing logic for PL/pgSQL dialect support.

Review Change Stack

Update the PL/pgSQL grammar generator so it matches the committed
plpgsql/grammar.js for drift-sensitive rules.

- emit alias declarations before variable declarations
- emit FETCH as optional FROM plus required cursor name
- add a lightweight generator drift test for decl_statement and stmt_fetch

This prevents future regeneration from undoing the current committed grammar
shape.
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 26aafd26-b720-464b-8f86-98345a4b4da5

📥 Commits

Reviewing files that changed from the base of the PR and between 33e140f and 88d3a31.

📒 Files selected for processing (2)
  • script/generate-plpgsql-grammar.js
  • script/generate-plpgsql-grammar.test.js

📝 Walkthrough

Walkthrough

This PR fixes drift between the PL/pgSQL grammar generator and the committed grammar by updating script/generate-plpgsql-grammar.js to emit correct rule ordering for alias declarations and correct token parsing for fetch statements, then adds a test to prevent future drift.

Changes

Grammar Generator Drift Fixes

Layer / File(s) Summary
Generator fixes for decl_statement and stmt_fetch
script/generate-plpgsql-grammar.js
decl_statement reorders alias declarations before variable declarations to prevent ALIAS from being misinterpreted as a type name. stmt_fetch changes from optional (kw_from any_identifier) pair to optional kw_from followed by required any_identifier, matching the committed grammar and supporting both cursor-only and cursor-with-FROM forms.
Drift detection test for grammar rules
script/generate-plpgsql-grammar.test.js
New test extracts and compares exact rule text for decl_statement and stmt_fetch from both the generator template and committed grammar, detecting drift to prevent fixes from being silently lost during future grammar regeneration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🐰 A grammar generator fixed with care,
Two rules now placed with proper flair,
Alias first, then fetch astray—
Tests ensure they'll always stay!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: updating the generator script to sync with the committed grammar, addressing the core issue of generator drift.
Linked Issues check ✅ Passed All coding objectives from issue #29 are met: alias declarations now precede variable declarations in decl_statement, stmt_fetch uses optional($.kw_from) followed by required identifier, and a drift test is added.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing generator drift: modifications to the generator template and addition of a drift detection test, with no changes to scanner or broader PL/pgSQL architecture.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@AlexS778

Copy link
Copy Markdown
Contributor Author

@gmr Hello, found small drift between generator and grammar.

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.

PL/pgSQL grammar generator has drifted from committed grammar.js

1 participant