Skip to content

fix(postgresql,kingbase): use SPI quoteIdentifierAlways for foreign schema and table in REFERENCES - #2265

Closed
Aias00 wants to merge 2 commits into
OtterMind:mainfrom
Aias00:rework/pg-kingbase-fk-via-spi
Closed

fix(postgresql,kingbase): use SPI quoteIdentifierAlways for foreign schema and table in REFERENCES#2265
Aias00 wants to merge 2 commits into
OtterMind:mainfrom
Aias00:rework/pg-kingbase-fk-via-spi

Conversation

@Aias00

@Aias00 Aias00 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces the direct " concatenation in the old PR #2187 with processor.quoteIdentifierAlways(), per the maintainer's architectural review. Built on the SPI extension #2234.

The buildForeignColum method in both PostgreSQLIndexTypeEnum and KingBaseIndexTypeEnum concatenated getForeignSchemaName() and getForeignTableName() raw into the REFERENCES clause, while the foreign columns below were already double-quoted. Now all components go through the processor:

// Before
script.append(tableIndex.getForeignSchemaName()).append(".");
script.append(tableIndex.getForeignTableName()).append(" ");

// After
script.append(PostgreSQLMetaData.POSTGRE_SQL_IDENTIFIER_PROCESSOR.quoteIdentifierAlways(tableIndex.getForeignSchemaName())).append(".");
script.append(PostgreSQLMetaData.POSTGRE_SQL_IDENTIFIER_PROCESSOR.quoteIdentifierAlways(tableIndex.getForeignTableName())).append(" ");

Verification

  • mvn compile -> BUILD SUCCESS (on the SPI extension branch)
  • Live PostgreSQL 16 verification on the old PR confirmed: quoted FK schema/table works for mixed-case names; unquoted fails.

Contributor declaration

  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below.

AI assistance: The fix, verification, and PR description were produced with Claude Code assistance.

Copilot AI review requested due to automatic review settings July 27, 2026 13:10
@Aias00
Aias00 requested a review from openai0229 as a code owner July 27, 2026 13:10

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@openai0229 openai0229 moved this to In Review in Chat2DB Community Jul 27, 2026
@Aias00
Aias00 force-pushed the rework/pg-kingbase-fk-via-spi branch from 9f470bf to e9df588 Compare July 27, 2026 13:12
@Aias00

Aias00 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the import — the processor classes needed correct package-qualified imports. Now compiles cleanly: mvn compile -> BUILD SUCCESS.

Aias00 and others added 2 commits July 27, 2026 09:09
Add a new SPI method that unconditionally quotes an identifier,
preserving the exact name regardless of case or keyword status.
Embedded delimiter characters are escaped per the dialect convention
(double-quote "" for ", backtick `` for `, bracket ]] for ]).

Fix removeIdentifierQuote to unescape doubled delimiters, satisfying
the round-trip contract: removeIdentifierQuote(quoteIdentifierAlways(raw)).equals(raw).

Implementations:
- DefaultSQLIdentifierProcessor: double-quote + escape " -> ""
- MysqlIdentifierProcessor: backtick + escape ` -> ``
- SqlServerIdentifierProcessor: bracket + escape ] -> ]]

Also add SNOWFLAKE_SQL_IDENTIFIER_PROCESSOR static field to
SnowflakeMetaData, mirroring PostgreSQL/KingBase, to give Snowflake
column-type enums access to the processor for downstream quoting PRs.

Tests: 36 tests covering conditional vs always-quote, embedded
delimiters, round-trip, reserved keywords, and case preservation
across Default, MySQL, and SqlServer processors.

Addresses the maintainer review on OtterMind#2187 requesting the shared SPI
contract be extended before rebasing the individual dialect quoting PRs.

Co-Authored-By: Claude <noreply@anthropic.com>
…chema and table in REFERENCES

Replaces direct " concatenation with the dialect-specific processor:
PostgreSQLMetaData.POSTGRE_SQL_IDENTIFIER_PROCESSOR and
KingBaseMetaData.KINGBASE_SQL_IDENTIFIER_PROCESSOR.
Built on the SPI extension (OtterMind#2234).

Closes OtterMind#2187 (superseded by this SPI-based approach)

Co-Authored-By: Claude <noreply@anthropic.com>

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This remains blocked on PR 2234 and only routes the foreign schema and table through the processor. Foreign column names are still directly wrapped with double quotes, despite the review requiring each qualified identifier component to be processed separately. After 2234 is resolved, rebase and add exact generated REFERENCES DDL tests for schema, table, and every column, including omitted schema, mixed case, reserved words, and embedded double quotes.

@Aias00
Aias00 force-pushed the rework/pg-kingbase-fk-via-spi branch from c47063d to cc660cb Compare July 28, 2026 14:24
@openai0229

Copy link
Copy Markdown
Contributor

Superseded by #2194 and #2197, which quote the foreign schema, table, and every referenced column for PostgreSQL and KingBase. This PR quotes only schema and table, conflicts with main, and has no remaining behavior to retain.

@openai0229 openai0229 closed this Jul 30, 2026
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants