feat: centralize quoting logic with PostgreSQL-compatible utilities #241
Conversation
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
- Remove hardcoded RESERVED_WORDS from quote-utils.ts - Import RESERVED_KEYWORDS and TYPE_FUNC_NAME_KEYWORDS from kwlist.ts - Add explicit Set<string> type annotations to kwlist.ts exports for proper TypeScript compatibility - QuoteUtils.needsQuotes() now uses the centralized keyword sets Co-Authored-By: Dan Lynch <[email protected]>
…ions The generated kwlist.ts needs explicit Set<string> type annotations for TypeScript compatibility when calling .has() with string arguments. Co-Authored-By: Dan Lynch <[email protected]>
…Utils - Remove Deparser.RESERVED_WORDS and Deparser.needsQuotes from deparser.ts - Add QuoteUtils.needsQuotesForString() and QuoteUtils.quoteString() methods - Update all call sites to use QuoteUtils instead of Deparser methods - Both QuoteUtils.needsQuotes and QuoteUtils.needsQuotesForString now use RESERVED_KEYWORDS from kwlist.ts as the single source of truth - Add Set<string> type annotations to kwlist.ts exports for TypeScript compatibility Co-Authored-By: Dan Lynch <[email protected]>
Co-Authored-By: Dan Lynch <[email protected]>
…L ruleutils.c Port PostgreSQL's quote_identifier() and quote_qualified_identifier() functions to TypeScript. These functions properly: - Quote identifiers only when needed (lowercase letters, digits, underscores) - Escape embedded double quotes as "" - Check against keyword categories (quote all except UNRESERVED_KEYWORD) References: - https://github.com/postgres/postgres/blob/fab5cd3dd1323f9e66efeb676c4bb212ff340204/src/backend/utils/adt/ruleutils.c#L13055-L13137 - https://github.com/postgres/postgres/blob/fab5cd3dd1323f9e66efeb676c4bb212ff340204/src/backend/utils/adt/ruleutils.c#L13139-L13156 Co-Authored-By: Dan Lynch <[email protected]>
devin-ai-integration
bot
changed the title
…precated methods - Replace all quoteString usages in deparser.ts with quoteIdentifier - Replace all needsQuotesForString usages with quoteIdentifier checks - Remove deprecated quoteString and needsQuotesForString methods from quote-utils.ts - Update quoteIfNeeded() and String() methods to use quoteIdentifier - Migrate DefElem contexts and VariableSetStmt to use quoteIdentifier The quoteIdentifier function is more correct because it: - Properly escapes embedded double quotes as "" - Checks all keyword categories (not just RESERVED_KEYWORDS) - Follows PostgreSQL's exact quote_identifier() logic from ruleutils.c Co-Authored-By: Dan Lynch <[email protected]>
…precated methods - Replace all QuoteUtils.quote() usages with QuoteUtils.quoteIdentifier() - Remove deprecated needsQuotes() and quote() methods from quote-utils.ts - QuoteUtils now only exports: escape, escapeEString, formatEString, needsEscapePrefix, quoteIdentifier, quoteQualifiedIdentifier This completes the migration to PostgreSQL-accurate identifier quoting. Co-Authored-By: Dan Lynch <[email protected]>
Type names like 'int' and 'timestamp' should not be quoted even though they are TYPE_FUNC_NAME_KEYWORDS. The quoteIdentifier function is meant for SQL identifiers (table names, column names, etc.), not for type names which have their own grammar rules in PostgreSQL. Co-Authored-By: Dan Lynch <[email protected]>
Replaced 11 instances of manual schema.name concatenation with quoteQualifiedIdentifier() for cleaner, more maintainable code: - RangeVar: schema.table patterns - DropStmt: operator class/family with schema - AlterObjectSchemaStmt: domain, type, conversion, parser, config, template, dictionary, operator class/family patterns Co-Authored-By: Dan Lynch <[email protected]>
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.
Summary
This PR centralizes and improves identifier quoting logic by:
Generated keyword list: Added
scripts/keywords.tsto generatekwlist.tsfrom PostgreSQL'skwlist.h, providing accurate keyword categorization (RESERVED_KEYWORD, UNRESERVED_KEYWORD, TYPE_FUNC_NAME_KEYWORD, COL_NAME_KEYWORD).New QuoteUtils methods:
quoteIdentifier()- TypeScript port of PostgreSQL'squote_identifier()from ruleutils.c, with proper escaping of embedded double quotes as""quoteQualifiedIdentifier()- TypeScript port of PostgreSQL'squote_qualified_identifier()Deparser refactor: Replaced inline quoting logic, hardcoded RESERVED_WORDS, and all legacy quoting methods with centralized
quoteIdentifier().Updates since last revision
QuoteUtils.quote()usages withQuoteUtils.quoteIdentifier()throughout deparser.tsquote(),needsQuotes(),quoteString(), andneedsQuotesForString()have been removed from quote-utils.tsint,timestamp) are no longer quoted even though they are TYPE_FUNC_NAME_KEYWORDS. Type names have their own grammar rules in PostgreSQL and should not be treated as identifiers.quoteIdentifier(schema) + '.' + quoteIdentifier(name)concatenation withquoteQualifiedIdentifier()for cleaner, more maintainable code (RangeVar, DropStmt, AlterObjectSchemaStmt contexts).Review & Testing Checklist for Human
quoteIdentifier()quotes all keywords except UNRESERVED_KEYWORD (e.g.,joinwill now be quoted as"join"), whereas the old logic only quoted RESERVED_KEYWORDS. This is more correct per PostgreSQL semantics but may change output.int,timestamp,intervalshould NOT be quoted in CREATE TABLE statements. The testdynamic creation of tablesvalidates this.quoteIdentifier()properly escapes"as"". Verify this works correctly in roundtrip tests (e.g., identifiera"bshould become"a""b").join,left,right), mixed-case names, and special characters.Test Plan
int,timestamp,interval, etc.)",$,.,-, or uppercase lettersNotes