Skip to content

Add minimal identifier escaping#856

Merged
mk3008 merged 1 commit into
mainfrom
codex/minimal-identifier-escape
May 30, 2026
Merged

Add minimal identifier escaping#856
mk3008 merged 1 commit into
mainfrom
codex/minimal-identifier-escape

Conversation

@mk3008
Copy link
Copy Markdown
Owner

@mk3008 mk3008 commented May 30, 2026

Summary

  • Add identifierEscapeTarget: "minimal" so SqlFormatter can remove identifier quotes only when the resulting bare token remains a plain identifier.
  • Keep quoted identifiers escaped for SQL special value expressions such as current_user, current_timestamp, session_user, and user, plus tokenizer-recognized keywords such as lateral, window, and typed-literal words such as date.
  • Preserve bare SQL special value expressions, while allowing qualified references such as users.current_user to parse as column references.

Verification

  • pnpm --filter rawsql-ts test -- tests/transformers/SqlFormatter.identifier-minimal.test.ts tests/transformers/FormatOptionResolver.test.ts tests/literal.test.ts tests/parsers/FullNameParser.test.ts
  • pnpm --filter rawsql-ts test
  • pnpm --filter rawsql-ts lint
  • pnpm --filter rawsql-ts build
  • pnpm --filter rawsql-ts benchmark
  • pnpm demo:complex-sql
  • git diff --check
  • Pre-commit pnpm test was attempted and failed outside this change in packages/ztd-cli / packages/transfer; the same workspace test failure reproduces on origin/main in C:\Users\mssgm\CodexApp\rawsql-ts-base-verify due missing built package entries for @rawsql-ts/testkit-postgres, @rawsql-ts/driver-adapter-core, and related ztd-cli lint failures.

Merge Readiness

  • No baseline exception requested.
  • Baseline exception requested and linked below.

Tracking issue: not needed; no baseline exception requested.
Scoped checks run: not needed; no baseline exception requested.
Why full baseline is not required: full baseline exception path not used for this PR.

Self Review

Self-review workflow: developer-self-review skill; consistency review and human acceptance review completed after implementation and verification.
Self-review result: no unresolved self-review blockers; tokenizer-dictionary drift found during review was fixed before PR.
Concept-review workflow: package boundary review against packages/core/AGENTS.md and DBMS-neutral parser/formatter scope.
Concept-review result: no unresolved concept or package-boundary violations; behavior remains parser/formatter-owned and does not branch by dialect.

CLI Surface Migration

  • No migration packet required for this CLI change.
  • CLI/user-facing surface change and migration packet completed.

No-migration rationale: not selected for this PR.
Upgrade note: not selected for this PR.
Deprecation/removal plan or issue: not selected for this PR.
Docs/help/examples updated: not selected for this PR.
Release/changeset wording: not selected for this PR.

Scaffold Contract Proof

  • No scaffold contract proof required for this PR.
  • Scaffold contract proof completed.

No-proof rationale: not selected for this PR.
Non-edit assertion: not selected for this PR.
Fail-fast input-contract proof: not selected for this PR.
Generated-output viability proof: not selected for this PR.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

Warning

Review limit reached

@mk3008, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 37 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8ac7a271-89f7-41e4-ae79-c8272427bb4c

📥 Commits

Reviewing files that changed from the base of the PR and between 27631c9 and 4698a87.

📒 Files selected for processing (11)
  • .changeset/minimal-identifier-escape.md
  • packages/core/src/parsers/FullNameParser.ts
  • packages/core/src/parsers/IdentifierDecorator.ts
  • packages/core/src/parsers/SqlPrintTokenParser.ts
  • packages/core/src/parsers/ValueParser.ts
  • packages/core/src/tokenReaders/LiteralTokenReader.ts
  • packages/core/src/transformers/FormatOptionResolver.ts
  • packages/core/src/transformers/SqlFormatter.ts
  • packages/core/src/utils/SqlSpecialValueKeywords.ts
  • packages/core/tests/transformers/FormatOptionResolver.test.ts
  • packages/core/tests/transformers/SqlFormatter.identifier-minimal.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/minimal-identifier-escape

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.

@mk3008 mk3008 merged commit 8c32567 into main May 30, 2026
10 checks passed
@mk3008 mk3008 deleted the codex/minimal-identifier-escape branch May 30, 2026 09:28
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.

1 participant