Skip to content

Fix Ignite connector to use standard SQL identifier quoting#29484

Open
yadavay-amzn wants to merge 1 commit into
trinodb:masterfrom
yadavay-amzn:fix/29303-ignite-identifier-quoting
Open

Fix Ignite connector to use standard SQL identifier quoting#29484
yadavay-amzn wants to merge 1 commit into
trinodb:masterfrom
yadavay-amzn:fix/29303-ignite-identifier-quoting

Conversation

@yadavay-amzn
Copy link
Copy Markdown

@yadavay-amzn yadavay-amzn commented May 14, 2026

Fixes #29303.

Problem

The Ignite connector uses backtick as identifier quote character, but Ignite SQL does not recognize backticks. This prevents creating tables with mixed-case identifiers.

Fix

Two changes:

  1. Change identifier quote from backtick to double-quote in IgniteClient. Ignite SQL requires double-quotes for case-sensitive identifiers per SQL standard.

  2. Override getRemoteIdentifiers() to force storesUpperCaseIdentifiers=true. Ignite stores unquoted identifiers as uppercase, but its JDBC driver does not report this correctly via DatabaseMetaData.storesUpperCaseIdentifiers(). The override ensures DefaultIdentifierMapping uppercases identifiers before quoting, so Trino's lowercase public becomes "PUBLIC" when sent to Ignite. This is the same pattern used by DruidJdbcClient.

Why the prior attempt (#29307) was closed

The prior contributor noted that swapping the quote character breaks schema resolution ("public" != PUBLIC). They missed that the IdentifierMapping infrastructure handles this, but, only when storesUpperCaseIdentifiers() returns true, which Ignite's driver does not report correctly. The getRemoteIdentifiers() override fixes this.

Testing

All Ignite plugin tests pass locally (361 tests):

  • TestIgniteClient (5/5) -- SQL generation with double-quote quoting
  • TestIgniteCaseInsensitiveMapping (10/10) -- case-insensitive identifier resolution
  • TestIgniteConnectorTest (346/346) -- full connector integration tests

@github-actions github-actions Bot added the ignite Ignite connector label May 14, 2026
@github-actions
Copy link
Copy Markdown

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

Copy link
Copy Markdown
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

Could you fix CI failures?

@yadavay-amzn yadavay-amzn force-pushed the fix/29303-ignite-identifier-quoting branch from 194360d to a1a52bd Compare May 14, 2026 23:18
Change identifier quote from backtick to double-quote in IgniteClient.
Ignite SQL requires double-quotes for case-sensitive identifiers.
The existing IdentifierMapping infrastructure handles the uppercase
folding (storesUpperCaseIdentifiers=true) automatically, matching
the pattern used by Oracle and Snowflake connectors.

Fixes trinodb#29303
@yadavay-amzn yadavay-amzn force-pushed the fix/29303-ignite-identifier-quoting branch from a1a52bd to 5eea2b2 Compare May 15, 2026 00:20
@yadavay-amzn
Copy link
Copy Markdown
Author

@ebyhr I fixed the test failures and pushed a new commit. CI is re-running for theses and i verified that the tests pass locally now.
Since this is my first trino change, i need my CLA verification to be approved so the CI might continue to fail until that is approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ignite Ignite connector

Development

Successfully merging this pull request may close these issues.

Ignite connector is unable to create tables with mixed-case identifiers

2 participants