From e21d5f8b89b4b733ba06586b608a0c42c395ce7d Mon Sep 17 00:00:00 2001 From: liuhy Date: Sun, 26 Jul 2026 22:44:37 -0700 Subject: [PATCH 1/2] feat(spi): add quoteIdentifierAlways to ISQLIdentifierProcessor 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 #2187 requesting the shared SPI contract be extended before rebasing the individual dialect quoting PRs. Co-Authored-By: Claude --- ...CompletionMetadataProviderAdapterTest.java | 8 ++ .../identifier/MysqlIdentifierProcessor.java | 16 ++- .../MysqlIdentifierProcessorTest.java | 70 +++++++++++ .../plugin/snowflake/SnowflakeMetaData.java | 4 + .../SqlServerIdentifierProcessor.java | 16 ++- .../SqlServerIdentifierProcessorTest.java | 69 +++++++++++ .../spi/DefaultSQLIdentifierProcessor.java | 13 ++- .../chat2db/spi/ISQLIdentifierProcessor.java | 19 +++ .../DefaultSQLIdentifierProcessorTest.java | 110 ++++++++++++++++++ 9 files changed, 322 insertions(+), 3 deletions(-) create mode 100644 chat2db-community-server/chat2db-community-plugins/chat2db-community-mysql/src/test/java/ai/chat2db/plugin/mysql/identifier/MysqlIdentifierProcessorTest.java create mode 100644 chat2db-community-server/chat2db-community-plugins/chat2db-community-sqlserver/src/test/java/ai/chat2db/plugin/sqlserver/identifier/SqlServerIdentifierProcessorTest.java create mode 100644 chat2db-community-server/chat2db-community-spi/src/test/java/ai/chat2db/spi/DefaultSQLIdentifierProcessorTest.java diff --git a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/completion/SqlCompletionMetadataProviderAdapterTest.java b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/completion/SqlCompletionMetadataProviderAdapterTest.java index c89ff7bf8e..a04c337c82 100644 --- a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/completion/SqlCompletionMetadataProviderAdapterTest.java +++ b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/completion/SqlCompletionMetadataProviderAdapterTest.java @@ -248,6 +248,14 @@ public String quoteIdentifier(String identifier) { return identifier == null ? null : "`" + identifier + "`"; } + @Override + public String quoteIdentifierAlways(String identifier) { + if (identifier == null) { + return null; + } + return "`" + identifier.replace("`", "``") + "`"; + } + @Override public String removeIdentifierQuote(String identifier) { return identifier == null ? null : identifier.replace("`", ""); diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-mysql/src/main/java/ai/chat2db/plugin/mysql/identifier/MysqlIdentifierProcessor.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-mysql/src/main/java/ai/chat2db/plugin/mysql/identifier/MysqlIdentifierProcessor.java index 5b83c4c18d..7e87ef42c8 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-mysql/src/main/java/ai/chat2db/plugin/mysql/identifier/MysqlIdentifierProcessor.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-mysql/src/main/java/ai/chat2db/plugin/mysql/identifier/MysqlIdentifierProcessor.java @@ -306,12 +306,26 @@ public String quoteIdentifier(String identifier) { return "`" + identifier + "`"; } + @Override + public String quoteIdentifierAlways(String identifier) { + if (identifier == null) { + return null; + } + return "`" + identifier.replace("`", "``") + "`"; + } + @Override public String removeIdentifierQuote(String identifier) { if (StringUtils.isBlank(identifier)) { return identifier; } - return removePattern(identifier, MYSQL_PATTERN); + if (identifier.startsWith("`") && identifier.endsWith("`") && identifier.length() >= 2) { + return identifier.substring(1, identifier.length() - 1).replace("``", "`"); + } + if (identifier.startsWith("\"") && identifier.endsWith("\"") && identifier.length() >= 2) { + return identifier.substring(1, identifier.length() - 1).replace("\"\"", "\""); + } + return identifier; } @Override diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-mysql/src/test/java/ai/chat2db/plugin/mysql/identifier/MysqlIdentifierProcessorTest.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-mysql/src/test/java/ai/chat2db/plugin/mysql/identifier/MysqlIdentifierProcessorTest.java new file mode 100644 index 0000000000..aba8fb9915 --- /dev/null +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-mysql/src/test/java/ai/chat2db/plugin/mysql/identifier/MysqlIdentifierProcessorTest.java @@ -0,0 +1,70 @@ +package ai.chat2db.plugin.mysql.identifier; + +import ai.chat2db.spi.DefaultSQLIdentifierProcessor; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * Contract tests for {@link MysqlIdentifierProcessor}. + * Covers the backtick-based {@code quoteIdentifierAlways} / + * {@code removeIdentifierQuote} round-trip and embedded-delimiter escaping. + */ +class MysqlIdentifierProcessorTest { + + private final MysqlIdentifierProcessor processor = new MysqlIdentifierProcessor(); + + @Test + void quoteIdentifierAlways_wrapsInBacktick() { + assertEquals("`mycol`", processor.quoteIdentifierAlways("mycol")); + } + + @Test + void quoteIdentifierAlways_escapesEmbeddedBacktick() { + assertEquals("`a``b`", processor.quoteIdentifierAlways("a`b")); + } + + @Test + void quoteIdentifierAlways_preservesMixedCase() { + assertEquals("`MixedCase`", processor.quoteIdentifierAlways("MixedCase")); + } + + @Test + void quoteIdentifierAlways_handlesNull() { + assertNull(processor.quoteIdentifierAlways(null)); + } + + @Test + void removeIdentifierQuote_stripsBacktick() { + assertEquals("mycol", processor.removeIdentifierQuote("`mycol`")); + } + + @Test + void removeIdentifierQuote_unescapesBacktick() { + assertEquals("a`b", processor.removeIdentifierQuote("`a``b`")); + } + + @Test + void removeIdentifierQuote_stripsDoubleQuote() { + assertEquals("mycol", processor.removeIdentifierQuote("\"mycol\"")); + } + + @Test + void roundTrip_plainName() { + String raw = "mycol"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } + + @Test + void roundTrip_embeddedBacktick() { + String raw = "a`b"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } + + @Test + void roundTrip_mixedCase() { + String raw = "MixedCase"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } +} diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-snowflake/src/main/java/ai/chat2db/plugin/snowflake/SnowflakeMetaData.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-snowflake/src/main/java/ai/chat2db/plugin/snowflake/SnowflakeMetaData.java index cd94c4b461..e4251b90dd 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-snowflake/src/main/java/ai/chat2db/plugin/snowflake/SnowflakeMetaData.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-snowflake/src/main/java/ai/chat2db/plugin/snowflake/SnowflakeMetaData.java @@ -3,6 +3,8 @@ import ai.chat2db.plugin.snowflake.builder.SnowflakeSqlBuilder; import ai.chat2db.plugin.snowflake.enums.type.*; import ai.chat2db.spi.IDbMetaData; +import ai.chat2db.spi.DefaultSQLIdentifierProcessor; +import ai.chat2db.spi.ISQLIdentifierProcessor; import ai.chat2db.spi.ISqlBuilder; import ai.chat2db.spi.DefaultMetaService; import ai.chat2db.community.domain.api.model.account.*; @@ -29,6 +31,8 @@ import static ai.chat2db.plugin.snowflake.constant.SnowflakeMetaDataConstants.*; public class SnowflakeMetaData extends DefaultMetaService implements IDbMetaData { + public static final ISQLIdentifierProcessor SNOWFLAKE_SQL_IDENTIFIER_PROCESSOR = new DefaultSQLIdentifierProcessor(); + @Override diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-sqlserver/src/main/java/ai/chat2db/plugin/sqlserver/identifier/SqlServerIdentifierProcessor.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-sqlserver/src/main/java/ai/chat2db/plugin/sqlserver/identifier/SqlServerIdentifierProcessor.java index 1c0df48daa..f74b41ab47 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-sqlserver/src/main/java/ai/chat2db/plugin/sqlserver/identifier/SqlServerIdentifierProcessor.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-sqlserver/src/main/java/ai/chat2db/plugin/sqlserver/identifier/SqlServerIdentifierProcessor.java @@ -219,12 +219,26 @@ public String quoteIdentifier(String identifier) { return "[" + identifier + "]"; } + @Override + public String quoteIdentifierAlways(String identifier) { + if (identifier == null) { + return null; + } + return "[" + identifier.replace("]", "]]") + "]"; + } + @Override public String removeIdentifierQuote(String identifier) { if (StringUtils.isBlank(identifier)) { return identifier; } - return removePattern(identifier, SQL_SERVER_PATTERN); + if (identifier.startsWith("[") && identifier.endsWith("]") && identifier.length() >= 2) { + return identifier.substring(1, identifier.length() - 1).replace("]]", "]"); + } + if (identifier.startsWith("\"") && identifier.endsWith("\"") && identifier.length() >= 2) { + return identifier.substring(1, identifier.length() - 1).replace("\"\"", "\""); + } + return identifier; } @Override diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-sqlserver/src/test/java/ai/chat2db/plugin/sqlserver/identifier/SqlServerIdentifierProcessorTest.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-sqlserver/src/test/java/ai/chat2db/plugin/sqlserver/identifier/SqlServerIdentifierProcessorTest.java new file mode 100644 index 0000000000..31c8249911 --- /dev/null +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-sqlserver/src/test/java/ai/chat2db/plugin/sqlserver/identifier/SqlServerIdentifierProcessorTest.java @@ -0,0 +1,69 @@ +package ai.chat2db.plugin.sqlserver.identifier; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * Contract tests for {@link SqlServerIdentifierProcessor}. + * Covers the bracket-based {@code quoteIdentifierAlways} / + * {@code removeIdentifierQuote} round-trip and embedded-delimiter escaping. + */ +class SqlServerIdentifierProcessorTest { + + private final SqlServerIdentifierProcessor processor = new SqlServerIdentifierProcessor(); + + @Test + void quoteIdentifierAlways_wrapsInBrackets() { + assertEquals("[mycol]", processor.quoteIdentifierAlways("mycol")); + } + + @Test + void quoteIdentifierAlways_preservesMixedCase() { + assertEquals("[MixedCase]", processor.quoteIdentifierAlways("MixedCase")); + } + + @Test + void quoteIdentifierAlways_escapesEmbeddedCloseBracket() { + assertEquals("[a]]b]", processor.quoteIdentifierAlways("a]b")); + } + + @Test + void quoteIdentifierAlways_handlesNull() { + assertNull(processor.quoteIdentifierAlways(null)); + } + + @Test + void removeIdentifierQuote_stripsBrackets() { + assertEquals("mycol", processor.removeIdentifierQuote("[mycol]")); + } + + @Test + void removeIdentifierQuote_unescapesCloseBracket() { + assertEquals("a]b", processor.removeIdentifierQuote("[a]]b]")); + } + + @Test + void removeIdentifierQuote_stripsDoubleQuote() { + assertEquals("mycol", processor.removeIdentifierQuote("\"mycol\"")); + } + + @Test + void roundTrip_plainName() { + String raw = "mycol"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } + + @Test + void roundTrip_embeddedCloseBracket() { + String raw = "a]b"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } + + @Test + void roundTrip_mixedCase() { + String raw = "MixedCase"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } +} diff --git a/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/DefaultSQLIdentifierProcessor.java b/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/DefaultSQLIdentifierProcessor.java index faa8b3b5fd..23fa275a95 100644 --- a/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/DefaultSQLIdentifierProcessor.java +++ b/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/DefaultSQLIdentifierProcessor.java @@ -41,12 +41,23 @@ public String quoteIdentifier(String identifier) { return StringUtils.wrap(identifier, '"'); } + @Override + public String quoteIdentifierAlways(String identifier) { + if (identifier == null) { + return null; + } + return "\"" + identifier.replace("\"", "\"\"") + "\""; + } + @Override public String removeIdentifierQuote(String identifier) { if (StringUtils.isBlank(identifier)) { return identifier; } - return removePattern(identifier, STANDARD_PATTERN); + if (identifier.startsWith("\"") && identifier.endsWith("\"") && identifier.length() >= 2) { + return identifier.substring(1, identifier.length() - 1).replace("\"\"", "\""); + } + return identifier; } @Override diff --git a/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/ISQLIdentifierProcessor.java b/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/ISQLIdentifierProcessor.java index d87387ea65..1664401984 100644 --- a/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/ISQLIdentifierProcessor.java +++ b/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/ISQLIdentifierProcessor.java @@ -87,4 +87,23 @@ public interface ISQLIdentifierProcessor { */ String escapeString(String str); + + /** + * Quotes an identifier unconditionally, preserving the exact name. + *

+ * Embedded delimiter characters are escaped per the dialect convention. + * Satisfies: {@code removeIdentifierQuote(quoteIdentifierAlways(raw)).equals(raw)} + *

+ * This is a default method for backward compatibility with external + * implementations that do not yet override it. The default delegates + * to {@link #quoteIdentifierIgnoreCase(String)} as a best-effort fallback; + * dialect implementations should override for correct always-quote semantics. + * + * @param identifier raw identifier text. + * @return unconditionally quoted identifier text with the original case preserved. + */ + default String quoteIdentifierAlways(String identifier) { + return quoteIdentifierIgnoreCase(identifier); + } + } diff --git a/chat2db-community-server/chat2db-community-spi/src/test/java/ai/chat2db/spi/DefaultSQLIdentifierProcessorTest.java b/chat2db-community-server/chat2db-community-spi/src/test/java/ai/chat2db/spi/DefaultSQLIdentifierProcessorTest.java new file mode 100644 index 0000000000..93198ac695 --- /dev/null +++ b/chat2db-community-server/chat2db-community-spi/src/test/java/ai/chat2db/spi/DefaultSQLIdentifierProcessorTest.java @@ -0,0 +1,110 @@ +package ai.chat2db.spi; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * Contract tests for {@link DefaultSQLIdentifierProcessor}. + * Covers the {@code quoteIdentifierAlways} / {@code removeIdentifierQuote} + * round-trip, conditional vs always-quote behavior, and embedded-delimiter + * escaping. + */ +class DefaultSQLIdentifierProcessorTest { + + private final DefaultSQLIdentifierProcessor processor = new DefaultSQLIdentifierProcessor(); + + // ---- quoteIdentifierAlways ---- + + @Test + void quoteIdentifierAlways_wrapsSimpleName() { + assertEquals("\"mycol\"", processor.quoteIdentifierAlways("mycol")); + } + + @Test + void quoteIdentifierAlways_preservesMixedCase() { + assertEquals("\"MixedCase\"", processor.quoteIdentifierAlways("MixedCase")); + } + + @Test + void quoteIdentifierAlways_escapesEmbeddedDoubleQuote() { + assertEquals("\"A\"\"B\"", processor.quoteIdentifierAlways("A\"B")); + } + + @Test + void quoteIdentifierAlways_handlesReservedKeyword() { + assertEquals("\"SELECT\"", processor.quoteIdentifierAlways("SELECT")); + } + + @Test + void quoteIdentifierAlways_handlesNull() { + assertNull(processor.quoteIdentifierAlways(null)); + } + + // ---- quoteIdentifier (conditional) ---- + + @Test + void quoteIdentifier_doesNotQuoteLowercaseValid() { + assertEquals("lower", processor.quoteIdentifier("lower")); + } + + @Test + void quoteIdentifier_quotesInvalidIdentifier() { + assertEquals("\"a b\"", processor.quoteIdentifier("a b")); + } + + @Test + void quoteIdentifierIgnoreCase_doesNotQuoteLowercase() { + assertEquals("lower", processor.quoteIdentifierIgnoreCase("lower")); + } + + // ---- removeIdentifierQuote ---- + + @Test + void removeIdentifierQuote_stripsAndUnescapes() { + assertEquals("A\"B", processor.removeIdentifierQuote("\"A\"\"B\"")); + } + + @Test + void removeIdentifierQuote_passesThroughUnquoted() { + assertEquals("plain", processor.removeIdentifierQuote("plain")); + } + + @Test + void removeIdentifierQuote_stripsSimpleQuoted() { + assertEquals("mycol", processor.removeIdentifierQuote("\"mycol\"")); + } + + // ---- round-trip ---- + + @Test + void roundTrip_plainName() { + String raw = "mycol"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } + + @Test + void roundTrip_mixedCase() { + String raw = "MixedCase"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } + + @Test + void roundTrip_embeddedDelimiter() { + String raw = "A\"B"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } + + @Test + void roundTrip_reservedKeyword() { + String raw = "SELECT"; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } + + @Test + void roundTrip_empty() { + String raw = ""; + assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); + } +} From 85fe1ff591c1623767ffc8a9d6565f4612a2cd31 Mon Sep 17 00:00:00 2001 From: zgq Date: Wed, 29 Jul 2026 11:13:48 +0800 Subject: [PATCH 2/2] fix(spi): enforce identifier quoting contract --- .../core/impl/db/DbSqlParserServiceImpl.java | 13 +++-- .../impl/db/GenericSqlCompletionEngine.java | 24 ++++---- ...CompletionMetadataProviderAdapterTest.java | 25 +++++---- .../db/GenericSqlCompletionEngineTest.java | 19 +++++++ .../impl/db/SqlParserServiceImplTest.java | 13 +++++ .../plugin/snowflake/SnowflakeMetaData.java | 4 -- .../chat2db/spi/ISQLIdentifierProcessor.java | 20 +++++-- .../DefaultSQLIdentifierProcessorTest.java | 56 +++++++++++++++++++ 8 files changed, 141 insertions(+), 33 deletions(-) create mode 100644 chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/impl/db/GenericSqlCompletionEngineTest.java diff --git a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/db/DbSqlParserServiceImpl.java b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/db/DbSqlParserServiceImpl.java index e6f7b3aa9d..2f5137a322 100644 --- a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/db/DbSqlParserServiceImpl.java +++ b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/db/DbSqlParserServiceImpl.java @@ -362,10 +362,7 @@ public SqlContextParser contextParser(DbSqlContextParserRequest sqlContextParser } for (Token token : tokensOnDefault) { String text = token.getText(); - if (text.startsWith(".")) { - text = text.substring(1); - } - text = sqlIdentifierProcessor.removeIdentifierQuote(text); + text = normalizeIdentifierToken(sqlIdentifierProcessor, text); if (tableMap.containsKey(text)) { SimpleTableColumnMapping simpleTableColumnMapping = new SimpleTableColumnMapping(); Table table = tableMap.get(text); @@ -504,6 +501,14 @@ public SqlContextParser contextParser(DbSqlContextParserRequest sqlContextParser } } + static String normalizeIdentifierToken(ISQLIdentifierProcessor processor, String tokenText) { + if (tokenText == null) { + return null; + } + String identifier = tokenText.startsWith(".") ? tokenText.substring(1) : tokenText; + return processor.removeIdentifierQuote(identifier); + } + @Override public List sqlHover(DbSqlHoverRequest sqlHoverParam) { try { diff --git a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/db/GenericSqlCompletionEngine.java b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/db/GenericSqlCompletionEngine.java index 7caa9b3435..fc8abc5f81 100644 --- a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/db/GenericSqlCompletionEngine.java +++ b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/main/java/ai/chat2db/community/domain/core/impl/db/GenericSqlCompletionEngine.java @@ -322,9 +322,9 @@ && matchesPattern(beforeSql + " " + afterSql, SELECT_TIP_COLUMN_FROM_PATTERN)) { } } tableName = split[split.length - 1]; - databaseName = sqlIdentifierProcessor.removeIdentifierQuote(databaseName); - schemaName = sqlIdentifierProcessor.removeIdentifierQuote(schemaName); - tableName = sqlIdentifierProcessor.removeIdentifierQuote(tableName); + databaseName = normalizeIdentifierPart(sqlIdentifierProcessor, databaseName); + schemaName = normalizeIdentifierPart(sqlIdentifierProcessor, schemaName); + tableName = normalizeIdentifierPart(sqlIdentifierProcessor, tableName); tableAlias = sqlIdentifierProcessor.quoteIdentifier(tableAliasEntry.getValue()); if (supportDatabase && StringUtils.isBlank(databaseName)) { continue; @@ -551,17 +551,17 @@ private List buildColumnCandidates(Map t databaseName = splitName[0]; } } - databaseName = processor.removeIdentifierQuote(databaseName); + databaseName = normalizeIdentifierPart(processor, databaseName); if (supportDatabase && StringUtils.isBlank(databaseName)) { continue; } - schemaName = processor.removeIdentifierQuote(schemaName); + schemaName = normalizeIdentifierPart(processor, schemaName); if (supportSchema && StringUtils.isBlank(schemaName)) { continue; } String lastIdentifier = splitName[length - 1]; boolean quoted = processor.isQuoteIdentifier(lastIdentifier); - table = processor.removeIdentifierQuote(lastIdentifier); + table = normalizeIdentifierPart(processor, lastIdentifier); if (!quoted) { table = processor.convertIdentifierCase(table); } @@ -645,7 +645,7 @@ private List buildIdentifierResult(CompletionInfo info, } String paramDatabaseName = param.getDatabaseName(); String paramSchemaName = param.getSchemaName(); - String lastName = processor.removeIdentifierQuote(names[names.length - 1]); + String lastName = normalizeIdentifierPart(processor, names[names.length - 1]); List databases = metaData.databases(connection); if (CollectionUtils.isNotEmpty(databases)) { for (Database database : databases) { @@ -678,7 +678,7 @@ private List buildIdentifierResult(CompletionInfo info, } String databaseName = names.length - 2 < 0 ? paramDatabaseName : names[names.length - 2]; - databaseName = processor.removeIdentifierQuote(databaseName); + databaseName = normalizeIdentifierPart(processor, databaseName); List schemas = metaData.schemas(connection, databaseName); if (CollectionUtils.isNotEmpty(schemas)) { for (Schema schema : schemas) { @@ -713,8 +713,8 @@ private List buildIdentifierResult(CompletionInfo info, if (supportSchema && StringUtils.isBlank(schemaName)) { return List.of(); } - databaseName = processor.removeIdentifierQuote(databaseName); - schemaName = processor.removeIdentifierQuote(schemaName); + databaseName = normalizeIdentifierPart(processor, databaseName); + schemaName = normalizeIdentifierPart(processor, schemaName); String tableKey = getTableKey(dataSourceId, databaseName, schemaName); List tables = MemoryCacheManage.get(tableKey); if (CollectionUtils.isEmpty(tables)) { @@ -791,6 +791,10 @@ private static SqlCompletionCandidate candidate(SqlCompletionCandidateTypeEnum t return SqlCompletionCandidate.of(type, label); } + static String normalizeIdentifierPart(ISQLIdentifierProcessor processor, String identifier) { + return identifier == null ? null : processor.removeIdentifierQuote(identifier); + } + private int resolveCursor(DbSqlCompletionGetRequest param) { if (param.getCursor() != null) { return Math.max(0, Math.min(param.getCursor(), Objects.toString(param.getSql(), "").length())); diff --git a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/completion/SqlCompletionMetadataProviderAdapterTest.java b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/completion/SqlCompletionMetadataProviderAdapterTest.java index a04c337c82..adcc72eeb5 100644 --- a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/completion/SqlCompletionMetadataProviderAdapterTest.java +++ b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/completion/SqlCompletionMetadataProviderAdapterTest.java @@ -40,6 +40,15 @@ class SqlCompletionMetadataProviderAdapterTest { private final SqlCompletionConverter converter = new SqlCompletionConverterImpl(); + @Test + void legacyIdentifierProcessorUsesCompatibleAlwaysQuoteDefault() { + BacktickIdentifierProcessor processor = new BacktickIdentifierProcessor(); + + Assertions.assertEquals("`a``b`", processor.quoteIdentifierAlways("a`b")); + Assertions.assertEquals("a`b", + processor.removeIdentifierQuote(processor.quoteIdentifierAlways("a`b"))); + } + @Test void listTablesUsesConverterAndPrefixFilter() { FakeMetaData metaData = new FakeMetaData(); @@ -245,20 +254,16 @@ public String quoteIdentifier(String identifier, Integer majorVersion, Integer m @Override public String quoteIdentifier(String identifier) { - return identifier == null ? null : "`" + identifier + "`"; - } - - @Override - public String quoteIdentifierAlways(String identifier) { - if (identifier == null) { - return null; - } - return "`" + identifier.replace("`", "``") + "`"; + return identifier == null ? null : "`" + identifier.replace("`", "``") + "`"; } @Override public String removeIdentifierQuote(String identifier) { - return identifier == null ? null : identifier.replace("`", ""); + if (identifier == null || identifier.length() < 2 + || !identifier.startsWith("`") || !identifier.endsWith("`")) { + return identifier; + } + return identifier.substring(1, identifier.length() - 1).replace("``", "`"); } @Override diff --git a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/impl/db/GenericSqlCompletionEngineTest.java b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/impl/db/GenericSqlCompletionEngineTest.java new file mode 100644 index 0000000000..1b0e22dcae --- /dev/null +++ b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/impl/db/GenericSqlCompletionEngineTest.java @@ -0,0 +1,19 @@ +package ai.chat2db.community.domain.core.impl.db; + +import ai.chat2db.spi.DefaultSQLIdentifierProcessor; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +class GenericSqlCompletionEngineTest { + + private final DefaultSQLIdentifierProcessor processor = new DefaultSQLIdentifierProcessor(); + + @Test + void normalizeIdentifierPartUnquotesOnlyOuterDelimiters() { + assertEquals("A\"B", GenericSqlCompletionEngine.normalizeIdentifierPart(processor, "\"A\"\"B\"")); + assertEquals("A\"B", GenericSqlCompletionEngine.normalizeIdentifierPart(processor, "A\"B")); + assertNull(GenericSqlCompletionEngine.normalizeIdentifierPart(processor, null)); + } +} diff --git a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/impl/db/SqlParserServiceImplTest.java b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/impl/db/SqlParserServiceImplTest.java index 402c7da980..88c8fa3db8 100644 --- a/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/impl/db/SqlParserServiceImplTest.java +++ b/chat2db-community-server/chat2db-community-domain/chat2db-community-domain-core/src/test/java/ai/chat2db/community/domain/core/impl/db/SqlParserServiceImplTest.java @@ -3,6 +3,7 @@ import ai.chat2db.community.domain.api.model.parser.statement.insert.InsertValueMapping; import ai.chat2db.community.domain.api.enums.parser.InsertValueMappingStatusEnum; import ai.chat2db.community.domain.api.model.db.SimpleInsertValueMapping; +import ai.chat2db.spi.DefaultSQLIdentifierProcessor; import org.antlr.v4.runtime.CommonToken; import org.antlr.v4.runtime.Token; import org.junit.jupiter.api.Assertions; @@ -13,6 +14,18 @@ class SqlParserServiceImplTest { + @Test + void normalizeIdentifierTokenPreservesEmbeddedQuotes() { + DefaultSQLIdentifierProcessor processor = new DefaultSQLIdentifierProcessor(); + + Assertions.assertEquals("A\"B", + DbSqlParserServiceImpl.normalizeIdentifierToken(processor, "\"A\"\"B\"")); + Assertions.assertEquals("A\"B", + DbSqlParserServiceImpl.normalizeIdentifierToken(processor, "A\"B")); + Assertions.assertEquals("Order", + DbSqlParserServiceImpl.normalizeIdentifierToken(processor, ".\"Order\"")); + } + @Test @SuppressWarnings("unchecked") void getSimpleInsertValueMappingsReturnsRowTokenRange() throws Exception { diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-snowflake/src/main/java/ai/chat2db/plugin/snowflake/SnowflakeMetaData.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-snowflake/src/main/java/ai/chat2db/plugin/snowflake/SnowflakeMetaData.java index e4251b90dd..cd94c4b461 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-snowflake/src/main/java/ai/chat2db/plugin/snowflake/SnowflakeMetaData.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-snowflake/src/main/java/ai/chat2db/plugin/snowflake/SnowflakeMetaData.java @@ -3,8 +3,6 @@ import ai.chat2db.plugin.snowflake.builder.SnowflakeSqlBuilder; import ai.chat2db.plugin.snowflake.enums.type.*; import ai.chat2db.spi.IDbMetaData; -import ai.chat2db.spi.DefaultSQLIdentifierProcessor; -import ai.chat2db.spi.ISQLIdentifierProcessor; import ai.chat2db.spi.ISqlBuilder; import ai.chat2db.spi.DefaultMetaService; import ai.chat2db.community.domain.api.model.account.*; @@ -31,8 +29,6 @@ import static ai.chat2db.plugin.snowflake.constant.SnowflakeMetaDataConstants.*; public class SnowflakeMetaData extends DefaultMetaService implements IDbMetaData { - public static final ISQLIdentifierProcessor SNOWFLAKE_SQL_IDENTIFIER_PROCESSOR = new DefaultSQLIdentifierProcessor(); - @Override diff --git a/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/ISQLIdentifierProcessor.java b/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/ISQLIdentifierProcessor.java index 1664401984..b83a4ee368 100644 --- a/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/ISQLIdentifierProcessor.java +++ b/chat2db-community-server/chat2db-community-spi/src/main/java/ai/chat2db/spi/ISQLIdentifierProcessor.java @@ -92,18 +92,28 @@ public interface ISQLIdentifierProcessor { * Quotes an identifier unconditionally, preserving the exact name. *

* Embedded delimiter characters are escaped per the dialect convention. - * Satisfies: {@code removeIdentifierQuote(quoteIdentifierAlways(raw)).equals(raw)} + * Implementations must satisfy: + * {@code removeIdentifierQuote(quoteIdentifierAlways(raw)).equals(raw)}. *

* This is a default method for backward compatibility with external - * implementations that do not yet override it. The default delegates - * to {@link #quoteIdentifierIgnoreCase(String)} as a best-effort fallback; - * dialect implementations should override for correct always-quote semantics. + * implementations that do not yet override it. The default accepts the + * result of {@link #quoteIdentifierIgnoreCase(String)} only when the + * implementation reports that result as quoted. Otherwise it fails fast + * instead of silently violating the always-quote contract. * * @param identifier raw identifier text. * @return unconditionally quoted identifier text with the original case preserved. */ default String quoteIdentifierAlways(String identifier) { - return quoteIdentifierIgnoreCase(identifier); + if (identifier == null) { + return null; + } + String quoted = quoteIdentifierIgnoreCase(identifier); + if (isQuoteIdentifier(quoted)) { + return quoted; + } + throw new UnsupportedOperationException( + "quoteIdentifierAlways must be implemented for this SQL dialect"); } } diff --git a/chat2db-community-server/chat2db-community-spi/src/test/java/ai/chat2db/spi/DefaultSQLIdentifierProcessorTest.java b/chat2db-community-server/chat2db-community-spi/src/test/java/ai/chat2db/spi/DefaultSQLIdentifierProcessorTest.java index 93198ac695..089545fa1b 100644 --- a/chat2db-community-server/chat2db-community-spi/src/test/java/ai/chat2db/spi/DefaultSQLIdentifierProcessorTest.java +++ b/chat2db-community-server/chat2db-community-spi/src/test/java/ai/chat2db/spi/DefaultSQLIdentifierProcessorTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * Contract tests for {@link DefaultSQLIdentifierProcessor}. @@ -107,4 +108,59 @@ void roundTrip_empty() { String raw = ""; assertEquals(raw, processor.removeIdentifierQuote(processor.quoteIdentifierAlways(raw))); } + + @Test + void legacyConditionalImplementationFailsFastInsteadOfReturningUnquotedIdentifier() { + ISQLIdentifierProcessor legacyProcessor = new LegacyConditionalIdentifierProcessor(); + + assertThrows(UnsupportedOperationException.class, + () -> legacyProcessor.quoteIdentifierAlways("plain")); + } + + private static final class LegacyConditionalIdentifierProcessor implements ISQLIdentifierProcessor { + @Override + public boolean isValidIdentifier(String identifier) { + return true; + } + + @Override + public boolean isReservedKeyword(String identifier, Integer majorVersion, Integer minorVersion) { + return false; + } + + @Override + public String quoteIdentifier(String identifier, Integer majorVersion, Integer minorVersion) { + return identifier; + } + + @Override + public String quoteIdentifier(String identifier) { + return identifier; + } + + @Override + public String removeIdentifierQuote(String identifier) { + return identifier; + } + + @Override + public String quoteIdentifierIgnoreCase(String identifier) { + return identifier; + } + + @Override + public boolean isQuoteIdentifier(String identifier) { + return false; + } + + @Override + public String convertIdentifierCase(String identifier) { + return identifier; + } + + @Override + public String escapeString(String str) { + return str; + } + } }