Skip to content

fix(kingbase): use SPI quoteIdentifierAlways for DROP COLUMN - #2261

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

fix(kingbase): use SPI quoteIdentifierAlways for DROP COLUMN#2261
Aias00 wants to merge 2 commits into
OtterMind:mainfrom
Aias00:rework/kingbase-drop-column-via-spi

Conversation

@Aias00

@Aias00 Aias00 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces the direct backtick concatenation in the old PR #2145 with KingBaseMetaData.KINGBASE_SQL_IDENTIFIER_PROCESSOR.quoteIdentifierAlways(name), per the maintainer's architectural review on #2187. Built on the SPI extension #2234.

KingBase is PostgreSQL-derived and uses double-quote identifiers. The old code used MySQL-style backticks for DROP COLUMN, producing invalid SQL. The constant is reverted to "DROP COLUMN " (no delimiter), and the column name is quoted via the SPI processor.

Verification

  • mvn compile -> BUILD SUCCESS (on the SPI extension branch)

Contributor declaration

  • I linked the Issue that defines this change.
  • 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.

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

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.

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>
Replaces direct backtick concatenation with
KingBaseMetaData.KINGBASE_SQL_IDENTIFIER_PROCESSOR.quoteIdentifierAlways,
per the maintainer review on OtterMind#2187. Built on the SPI extension (OtterMind#2234).

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

Co-Authored-By: Claude <noreply@anthropic.com>
@Aias00
Aias00 force-pushed the rework/kingbase-drop-column-via-spi branch from 0861412 to 032c404 Compare July 27, 2026 16:09

@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 currently has compile-only verification for the KingBase behavior. After the SPI contract is resolved and merged, rebase so this PR contains only the KingBase delta and add generated-DDL tests for DROP COLUMN with a normal name, mixed case or reserved word, and an embedded double quote. The exact output must prove that delimiter escaping comes from the processor.

@openai0229

Copy link
Copy Markdown
Contributor

Superseded by #2197 (b4a900d), which already applies the KingBase identifier processor to DROP COLUMN and covers the broader ALTER paths. This PR now conflicts with main and has no additional 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