fix: add 13 missing reserved keywords to identifier quoting (#328)#330
fix: add 13 missing reserved keywords to identifier quoting (#328)#330
Conversation
Greptile SummaryThis PR fixes a critical bug where 13 PostgreSQL reserved keywords were not being quoted when used as column identifiers in Key Changes:
Impact: Confidence Score: 5/5
Important Files Changed
Last reviewed commit: b82af81 |
There was a problem hiding this comment.
Pull request overview
Fixes invalid SQL emitted by pgschema dump when PostgreSQL reserved keywords are used as column identifiers by ensuring those identifiers are quoted consistently.
Changes:
- Add 13 missing PostgreSQL reserved keywords to
ir/quote.go’sreservedWordsmap so they get quoted when used as identifiers. - Add a new dump integration regression test case (testdata +
TestDumpCommand_Issue328MissingReservedKeywords) covering all 13 keywords as column names. - Update Claude/skill documentation and add local PostgreSQL lexer reference (
internal/scan.l) to support future grammar/keyword investigations.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
ir/quote.go |
Extends reservedWords with the 13 missing keywords so needsQuoting triggers quoting. |
cmd/dump/dump_integration_test.go |
Adds an integration test to ensure dump output quotes the newly-added reserved keywords. |
testdata/dump/issue_328_missing_reserved_keywords/raw.sql |
New raw schema fixture using all 13 keywords as quoted column names. |
testdata/dump/issue_328_missing_reserved_keywords/pgschema.sql |
New expected pgschema dump output verifying quoted identifiers. |
testdata/dump/issue_328_missing_reserved_keywords/pgdump.sql |
New pg_dump-based setup SQL to reproduce the schema in tests. |
testdata/dump/issue_328_missing_reserved_keywords/manifest.json |
Metadata for the new dump regression test suite. |
internal/scan.l |
Adds a local copy of PostgreSQL’s lexer source as a reference artifact. |
CLAUDE.md |
Updates repository guidance, including references to local Postgres sources and dependencies. |
.claude/skills/run_tests/SKILL.md |
Updates testing guidance to reflect diff.sql/plan files naming. |
.claude/skills/postgres_syntax/SKILL.md |
Updates syntax-reference guidance to prefer local internal/gram.y / internal/scan.l. |
.claude/skills/fix_bug/SKILL.md |
Notes that --generate produces plan artifacts alongside diff.sql. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b82af81 to
ffba098
Compare
Add analyse, analyze, asc, concurrently, desc, full, leading, localtime, localtimestamp, notnull, overlaps, placing, and session_user to the reservedWords map in ir/quote.go. These are classified as RESERVED_KEYWORD or TYPE_FUNC_NAME_KEYWORD in PostgreSQL's gram.y and require quoting when used as column identifiers. Fixes #328 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ffba098 to
7983f66
Compare
Summary
pgschema dumpwas not quoting 13 PostgreSQL reserved keywords when used as column identifiers, producing invalid SQL that fails on re-apply.The
reservedWordsmap inir/quote.gowas missing these keywords that are classified asRESERVED_KEYWORDorTYPE_FUNC_NAME_KEYWORDin PostgreSQL'sgram.y:analyse,analyze,asc,concurrently,desc,full,leading,localtime,localtimestamp,notnull,overlaps,placing,session_userThe fix adds all 13 keywords to the map, cross-referenced against PostgreSQL's
gram.ygrammar file.Fixes #328
Test plan
TestDumpCommand_Issue328MissingReservedKeywordswith a table using all 13 keywords as column namesgo test -v ./cmd/dump -run TestDumpCommand_Issue328🤖 Generated with Claude Code