From c8f4068673ee82e00bbf362d6e476d1754c5601e Mon Sep 17 00:00:00 2001 From: HandSonic <8078023+handsonic@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:04:21 +0800 Subject: [PATCH 1/5] fix(redshift): escape SQL identifiers and literals in metadata/DDL paths (#1914) --- .../plugin/redshift/RedshiftMetaData.java | 8 ++- .../plugin/redshift/RedshiftSqlEscapes.java | 47 ++++++++++++++ .../redshift/RedshiftSqlEscapesTest.java | 61 +++++++++++++++++++ 3 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapes.java create mode 100644 chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapesTest.java diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java index bc90f15865..e69af3f162 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java @@ -20,8 +20,7 @@ public class RedshiftMetaData extends PostgreSQLMetaData implements IDbMetaData { @Override public String tableDDL(Connection connection, String databaseName, String schemaName, String tableName) { - String sql = "SHOW CREATE TABLE " + format(schemaName) + "." - + format(tableName); + String sql = buildShowCreateTableSql(schemaName, tableName); return DefaultSQLExecutor.getInstance().execute(connection, sql, resultSet -> { if (resultSet.next()) { return resultSet.getString(1); @@ -30,6 +29,11 @@ public String tableDDL(Connection connection, String databaseName, String schema }); } + static String buildShowCreateTableSql(String schemaName, String tableName) { + return "SHOW CREATE TABLE " + RedshiftSqlEscapes.quoteIdentifier(schemaName) + "." + + RedshiftSqlEscapes.quoteIdentifier(tableName); + } + @Override public List indexes(Connection connection, String databaseName, String schemaName, String tableName) { return DefaultSQLExecutor.getInstance().indexes(connection, StringUtils.isEmpty(databaseName) ? null : databaseName, StringUtils.isEmpty(schemaName) ? null : schemaName, tableName); diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapes.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapes.java new file mode 100644 index 0000000000..b330dd804b --- /dev/null +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapes.java @@ -0,0 +1,47 @@ +package ai.chat2db.plugin.redshift; + +import org.apache.commons.lang3.StringUtils; + +/** + * Escaping helpers for values and identifiers interpolated into Redshift SQL. + */ +public final class RedshiftSqlEscapes { + + private static final String DOUBLE_QUOTE = "\""; + + private RedshiftSqlEscapes() { + } + + /** + * Escapes a value interpolated into a single-quoted SQL string literal + * by doubling every single quote. + */ + public static String escapeSqlLiteral(String value) { + return value == null ? null : StringUtils.replace(value, "'", "''"); + } + + /** + * Escapes a double-quoted identifier by stripping one surrounding quote pair + * and doubling every embedded double quote. + */ + public static String escapeIdentifier(String identifier) { + if (identifier == null) { + return null; + } + String unquoted = identifier; + if (unquoted.length() >= 2 && unquoted.startsWith(DOUBLE_QUOTE) && unquoted.endsWith(DOUBLE_QUOTE)) { + unquoted = unquoted.substring(1, unquoted.length() - 1); + } + return StringUtils.replace(unquoted, "\"", "\"\""); + } + + /** + * Wraps an identifier in double quotes with embedded quotes doubled. + */ + public static String quoteIdentifier(String identifier) { + if (StringUtils.isBlank(identifier)) { + return identifier; + } + return DOUBLE_QUOTE + escapeIdentifier(identifier) + DOUBLE_QUOTE; + } +} diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapesTest.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapesTest.java new file mode 100644 index 0000000000..2ae15a65a7 --- /dev/null +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapesTest.java @@ -0,0 +1,61 @@ +package ai.chat2db.plugin.redshift; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class RedshiftSqlEscapesTest { + + @Test + void escapeSqlLiteralDoublesSingleQuotes() { + assertEquals("o''brien", RedshiftSqlEscapes.escapeSqlLiteral("o'brien")); + assertEquals("plain", RedshiftSqlEscapes.escapeSqlLiteral("plain")); + assertEquals("''''", RedshiftSqlEscapes.escapeSqlLiteral("''")); + assertNull(RedshiftSqlEscapes.escapeSqlLiteral(null)); + } + + @Test + void escapeIdentifierDoublesEmbeddedDoubleQuotes() { + assertEquals("we\"\"ird", RedshiftSqlEscapes.escapeIdentifier("we\"ird")); + assertEquals("plain", RedshiftSqlEscapes.escapeIdentifier("plain")); + assertNull(RedshiftSqlEscapes.escapeIdentifier(null)); + } + + @Test + void escapeIdentifierStripsOneSurroundingQuotePair() { + assertEquals("foo", RedshiftSqlEscapes.escapeIdentifier("\"foo\"")); + assertEquals("fo\"\"o", RedshiftSqlEscapes.escapeIdentifier("\"fo\"o\"")); + } + + @Test + void quoteIdentifierWrapsAndEscapes() { + assertEquals("\"foo\"", RedshiftSqlEscapes.quoteIdentifier("foo")); + assertEquals("\"foo\"", RedshiftSqlEscapes.quoteIdentifier("\"foo\"")); + assertEquals("\"we\"\"ird\"", RedshiftSqlEscapes.quoteIdentifier("we\"ird")); + assertEquals("", RedshiftSqlEscapes.quoteIdentifier("")); + assertNull(RedshiftSqlEscapes.quoteIdentifier(null)); + } + + @Test + void maliciousSchemaNameIsNeutralizedInShowCreateTable() { + String sql = RedshiftMetaData.buildShowCreateTableSql("public\"; DROP TABLE users; --", "t"); + assertEquals("SHOW CREATE TABLE \"public\"\"; DROP TABLE users; --\".\"t\"", sql); + } + + @Test + void maliciousTableNameIsNeutralizedInShowCreateTable() { + String sql = RedshiftMetaData.buildShowCreateTableSql("public", "t\" OR \"1\"=\"1"); + assertEquals("SHOW CREATE TABLE \"public\".\"t\"\" OR \"\"1\"\"=\"\"1\"", sql); + assertTrue(sql.startsWith("SHOW CREATE TABLE \"public\".")); + } + + @Test + void benignNamesProduceSameSqlAsBefore() { + assertEquals("SHOW CREATE TABLE \"public\".\"orders\"", + RedshiftMetaData.buildShowCreateTableSql("public", "orders")); + assertEquals("SHOW CREATE TABLE \"public\".\"orders\"", + RedshiftMetaData.buildShowCreateTableSql("\"public\"", "\"orders\"")); + } +} From 34990170a2b08d4c1611e32303065216ab95b721 Mon Sep 17 00:00:00 2001 From: HandSonic <8078023+handsonic@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:52:24 +0800 Subject: [PATCH 2/5] refactor(redshift): move escaping into RedshiftIdentifierProcessor per maintainer review (#1914) - new RedshiftIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier with double-quote doubling, escapeString with single-quote doubling - RedshiftMetaData overrides getSQLIdentifierProcessor(); SHOW CREATE TABLE call site uses RedshiftIdentifierProcessor.INSTANCE - RedshiftSqlEscapes removed; tests migrated (9 green) --- .../plugin/redshift/RedshiftMetaData.java | 12 +++- .../plugin/redshift/RedshiftSqlEscapes.java | 47 -------------- .../RedshiftIdentifierProcessor.java | 65 +++++++++++++++++++ ...a => RedshiftIdentifierProcessorTest.java} | 31 ++++----- 4 files changed, 91 insertions(+), 64 deletions(-) delete mode 100644 chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapes.java create mode 100644 chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java rename chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/{RedshiftSqlEscapesTest.java => RedshiftIdentifierProcessorTest.java} (52%) diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java index e69af3f162..117a142adf 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java @@ -1,12 +1,14 @@ package ai.chat2db.plugin.redshift; import ai.chat2db.plugin.postgresql.PostgreSQLMetaData; +import ai.chat2db.plugin.redshift.identifier.RedshiftIdentifierProcessor; import ai.chat2db.spi.IDbMetaData; import ai.chat2db.community.domain.api.model.metadata.Function; import ai.chat2db.community.domain.api.model.metadata.Procedure; import ai.chat2db.community.domain.api.model.metadata.TableIndex; import ai.chat2db.community.domain.api.model.metadata.Trigger; import ai.chat2db.spi.DefaultSQLExecutor; +import ai.chat2db.spi.ISQLIdentifierProcessor; import jakarta.validation.constraints.NotEmpty; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; @@ -18,6 +20,12 @@ import static ai.chat2db.plugin.redshift.constant.RedshiftMetaDataConstants.*; @Slf4j public class RedshiftMetaData extends PostgreSQLMetaData implements IDbMetaData { + + @Override + public ISQLIdentifierProcessor getSQLIdentifierProcessor() { + return RedshiftIdentifierProcessor.INSTANCE; + } + @Override public String tableDDL(Connection connection, String databaseName, String schemaName, String tableName) { String sql = buildShowCreateTableSql(schemaName, tableName); @@ -30,8 +38,8 @@ public String tableDDL(Connection connection, String databaseName, String schema } static String buildShowCreateTableSql(String schemaName, String tableName) { - return "SHOW CREATE TABLE " + RedshiftSqlEscapes.quoteIdentifier(schemaName) + "." - + RedshiftSqlEscapes.quoteIdentifier(tableName); + return "SHOW CREATE TABLE " + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier(schemaName) + "." + + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier(tableName); } @Override diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapes.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapes.java deleted file mode 100644 index b330dd804b..0000000000 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapes.java +++ /dev/null @@ -1,47 +0,0 @@ -package ai.chat2db.plugin.redshift; - -import org.apache.commons.lang3.StringUtils; - -/** - * Escaping helpers for values and identifiers interpolated into Redshift SQL. - */ -public final class RedshiftSqlEscapes { - - private static final String DOUBLE_QUOTE = "\""; - - private RedshiftSqlEscapes() { - } - - /** - * Escapes a value interpolated into a single-quoted SQL string literal - * by doubling every single quote. - */ - public static String escapeSqlLiteral(String value) { - return value == null ? null : StringUtils.replace(value, "'", "''"); - } - - /** - * Escapes a double-quoted identifier by stripping one surrounding quote pair - * and doubling every embedded double quote. - */ - public static String escapeIdentifier(String identifier) { - if (identifier == null) { - return null; - } - String unquoted = identifier; - if (unquoted.length() >= 2 && unquoted.startsWith(DOUBLE_QUOTE) && unquoted.endsWith(DOUBLE_QUOTE)) { - unquoted = unquoted.substring(1, unquoted.length() - 1); - } - return StringUtils.replace(unquoted, "\"", "\"\""); - } - - /** - * Wraps an identifier in double quotes with embedded quotes doubled. - */ - public static String quoteIdentifier(String identifier) { - if (StringUtils.isBlank(identifier)) { - return identifier; - } - return DOUBLE_QUOTE + escapeIdentifier(identifier) + DOUBLE_QUOTE; - } -} diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java new file mode 100644 index 0000000000..a1600b9f48 --- /dev/null +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java @@ -0,0 +1,65 @@ +package ai.chat2db.plugin.redshift.identifier; + +import ai.chat2db.spi.DefaultSQLIdentifierProcessor; +import org.apache.commons.lang3.StringUtils; + +/** + * Redshift dialect identifier processor: double-quoted identifiers with embedded-quote + * doubling, and single-quote doubling for string literals. Shared stateless + * instance available via {@link #INSTANCE} for call sites without MetaData access. + */ +public class RedshiftIdentifierProcessor extends DefaultSQLIdentifierProcessor { + + public static final RedshiftIdentifierProcessor INSTANCE = new RedshiftIdentifierProcessor(); + + /** + * Quotes with double quotes, stripping one surrounding quote pair and doubling + * every embedded double quote. Blank identifiers are returned unchanged. + */ + @Override + public String quoteIdentifier(String identifier) { + if (StringUtils.isBlank(identifier)) { + return identifier; + } + return "\"" + escapeIdentifierContent(identifier) + "\""; + } + + @Override + public String quoteIdentifier(String identifier, Integer majorVersion, Integer minorVersion) { + return quoteIdentifier(identifier); + } + + @Override + public String quoteIdentifierIgnoreCase(String identifier) { + return quoteIdentifier(identifier); + } + + /** + * Escapes a value interpolated into a single-quoted SQL string literal by + * doubling every single quote. + */ + @Override + public String escapeString(String str) { + return str == null ? null : StringUtils.replace(str, "'", "''"); + } + + private static String escapeIdentifierContent(String identifier) { + String stripped = identifier; + if (stripped.length() >= 2 && stripped.startsWith("\"") && stripped.endsWith("\"")) { + stripped = stripped.substring(1, stripped.length() - 1); + } + return StringUtils.replace(stripped, "\"", "\"\""); + } + + /** + * Escapes identifier content for a position already surrounded by double + * quotes: strips one surrounding quote pair, then doubles every embedded + * double quote. + */ + public static String escapeIdentifier(String identifier) { + if (identifier == null) { + return null; + } + return escapeIdentifierContent(identifier); + } +} diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapesTest.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java similarity index 52% rename from chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapesTest.java rename to chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java index 2ae15a65a7..d75c67d2e6 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftSqlEscapesTest.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java @@ -1,41 +1,42 @@ package ai.chat2db.plugin.redshift; +import ai.chat2db.plugin.redshift.identifier.RedshiftIdentifierProcessor; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -class RedshiftSqlEscapesTest { +class RedshiftIdentifierProcessorTest { @Test void escapeSqlLiteralDoublesSingleQuotes() { - assertEquals("o''brien", RedshiftSqlEscapes.escapeSqlLiteral("o'brien")); - assertEquals("plain", RedshiftSqlEscapes.escapeSqlLiteral("plain")); - assertEquals("''''", RedshiftSqlEscapes.escapeSqlLiteral("''")); - assertNull(RedshiftSqlEscapes.escapeSqlLiteral(null)); + assertEquals("o''brien", RedshiftIdentifierProcessor.INSTANCE.escapeString("o'brien")); + assertEquals("plain", RedshiftIdentifierProcessor.INSTANCE.escapeString("plain")); + assertEquals("''''", RedshiftIdentifierProcessor.INSTANCE.escapeString("''")); + assertNull(RedshiftIdentifierProcessor.INSTANCE.escapeString(null)); } @Test void escapeIdentifierDoublesEmbeddedDoubleQuotes() { - assertEquals("we\"\"ird", RedshiftSqlEscapes.escapeIdentifier("we\"ird")); - assertEquals("plain", RedshiftSqlEscapes.escapeIdentifier("plain")); - assertNull(RedshiftSqlEscapes.escapeIdentifier(null)); + assertEquals("we\"\"ird", RedshiftIdentifierProcessor.escapeIdentifier("we\"ird")); + assertEquals("plain", RedshiftIdentifierProcessor.escapeIdentifier("plain")); + assertNull(RedshiftIdentifierProcessor.escapeIdentifier(null)); } @Test void escapeIdentifierStripsOneSurroundingQuotePair() { - assertEquals("foo", RedshiftSqlEscapes.escapeIdentifier("\"foo\"")); - assertEquals("fo\"\"o", RedshiftSqlEscapes.escapeIdentifier("\"fo\"o\"")); + assertEquals("foo", RedshiftIdentifierProcessor.escapeIdentifier("\"foo\"")); + assertEquals("fo\"\"o", RedshiftIdentifierProcessor.escapeIdentifier("\"fo\"o\"")); } @Test void quoteIdentifierWrapsAndEscapes() { - assertEquals("\"foo\"", RedshiftSqlEscapes.quoteIdentifier("foo")); - assertEquals("\"foo\"", RedshiftSqlEscapes.quoteIdentifier("\"foo\"")); - assertEquals("\"we\"\"ird\"", RedshiftSqlEscapes.quoteIdentifier("we\"ird")); - assertEquals("", RedshiftSqlEscapes.quoteIdentifier("")); - assertNull(RedshiftSqlEscapes.quoteIdentifier(null)); + assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("foo")); + assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("\"foo\"")); + assertEquals("\"we\"\"ird\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("we\"ird")); + assertEquals("", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("")); + assertNull(RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier(null)); } @Test From d2b2f105515f85e5e4bda939bfdf3e00c988491b Mon Sep 17 00:00:00 2001 From: HandSonic <8078023+handsonic@users.noreply.github.com> Date: Mon, 27 Jul 2026 03:51:04 +0800 Subject: [PATCH 3/5] fix(redshift): keep SPI quoteIdentifier conditional, reserve always-quote for DDL paths (#1914) - quoteIdentifier(String) is conditional again: null/blank passthrough, valid plain identifiers returned unquoted, otherwise wrapped with double quotes after stripping one pair and doubling embedded quotes - new quoteIdentifierAlways(String) for DDL-generation sites; buildShowCreateTableSql uses it (was RedshiftSqlEscapes.quoteIdentifier) - quoteIdentifierIgnoreCase keeps the always-quote SPI variant meaning; versioned overload delegates to quoteIdentifier(String) - tests cover both conditional and always behaviors incl. null passthrough --- .../plugin/redshift/RedshiftMetaData.java | 4 +- .../RedshiftIdentifierProcessor.java | 38 +++++++++++++-- .../RedshiftIdentifierProcessorTest.java | 47 +++++++++++++++++-- 3 files changed, 79 insertions(+), 10 deletions(-) diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java index 117a142adf..6ae56b0872 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java @@ -38,8 +38,8 @@ public String tableDDL(Connection connection, String databaseName, String schema } static String buildShowCreateTableSql(String schemaName, String tableName) { - return "SHOW CREATE TABLE " + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier(schemaName) + "." - + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier(tableName); + return "SHOW CREATE TABLE " + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(schemaName) + "." + + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(tableName); } @Override diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java index a1600b9f48..453c8e52db 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java @@ -7,21 +7,32 @@ * Redshift dialect identifier processor: double-quoted identifiers with embedded-quote * doubling, and single-quote doubling for string literals. Shared stateless * instance available via {@link #INSTANCE} for call sites without MetaData access. + * + *

{@link #quoteIdentifier(String)} follows the SPI contract: it is conditional + * and only quotes when the identifier is not already a valid plain identifier + * (or is a reserved keyword). {@link #quoteIdentifierAlways(String)} is the + * unconditional variant reserved for DDL-generation call sites that historically + * always produced quoted output. */ public class RedshiftIdentifierProcessor extends DefaultSQLIdentifierProcessor { public static final RedshiftIdentifierProcessor INSTANCE = new RedshiftIdentifierProcessor(); /** - * Quotes with double quotes, stripping one surrounding quote pair and doubling - * every embedded double quote. Blank identifiers are returned unchanged. + * SPI-facing conditional quoting: returns {@code null} for {@code null}, + * blank input unchanged, a valid plain identifier that is not a reserved + * keyword unquoted, and otherwise wraps with double quotes after stripping + * one surrounding quote pair and doubling every embedded double quote. */ @Override public String quoteIdentifier(String identifier) { if (StringUtils.isBlank(identifier)) { return identifier; } - return "\"" + escapeIdentifierContent(identifier) + "\""; + if (isValidIdentifier(identifier) && !isReservedKeyword(identifier.toUpperCase(), null, null)) { + return identifier; + } + return quoteIdentifierAlways(identifier); } @Override @@ -29,9 +40,28 @@ public String quoteIdentifier(String identifier, Integer majorVersion, Integer m return quoteIdentifier(identifier); } + /** + * SPI "preserve case, always quote" variant: unconditionally wraps with + * double quotes (blank input returned unchanged). + */ @Override public String quoteIdentifierIgnoreCase(String identifier) { - return quoteIdentifier(identifier); + if (StringUtils.isBlank(identifier)) { + return identifier; + } + return quoteIdentifierAlways(identifier); + } + + /** + * Unconditional quoting for DDL-generation call sites: {@code null} returns + * {@code null}, otherwise wraps with double quotes after stripping one + * surrounding quote pair and doubling every embedded double quote. + */ + public String quoteIdentifierAlways(String identifier) { + if (identifier == null) { + return null; + } + return "\"" + escapeIdentifierContent(identifier) + "\""; } /** diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java index d75c67d2e6..84f95b00a4 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java @@ -31,12 +31,51 @@ void escapeIdentifierStripsOneSurroundingQuotePair() { } @Test - void quoteIdentifierWrapsAndEscapes() { - assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("foo")); - assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("\"foo\"")); + void quoteIdentifierReturnsPlainIdentifiersUnquoted() { + assertEquals("foo", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("foo")); + assertEquals("orders_2", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("orders_2")); + } + + @Test + void quoteIdentifierQuotesWhenNotPlain() { assertEquals("\"we\"\"ird\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("we\"ird")); - assertEquals("", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("")); + assertEquals("\"has space\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("has space")); + assertEquals("\"2leading\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("2leading")); + } + + @Test + void quoteIdentifierRequotesAlreadyQuotedInput() { + assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("\"foo\"")); + } + + @Test + void quoteIdentifierPassesThroughNullAndBlank() { assertNull(RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier(null)); + assertEquals("", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("")); + } + + @Test + void versionedQuoteIdentifierDelegatesToConditional() { + assertEquals("foo", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("foo", null, null)); + assertEquals("\"has space\"", + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("has space", 1, 0)); + } + + @Test + void quoteIdentifierIgnoreCaseAlwaysQuotes() { + assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase("foo")); + assertEquals("\"we\"\"ird\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase("we\"ird")); + assertEquals("", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase("")); + assertNull(RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase(null)); + } + + @Test + void quoteIdentifierAlwaysWrapsAndEscapes() { + assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("foo")); + assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("\"foo\"")); + assertEquals("\"we\"\"ird\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("we\"ird")); + assertEquals("\"\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("")); + assertNull(RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(null)); } @Test From 5924ff4ac9388fd663c173bb2b6811f2138a04f8 Mon Sep 17 00:00:00 2001 From: HandSonic <8078023+handsonic@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:57:56 +0800 Subject: [PATCH 4/5] fix(redshift): blank passthrough in quoteIdentifierAlways (no empty quoted identifier) (#1914) --- .../redshift/identifier/RedshiftIdentifierProcessor.java | 8 ++++---- .../plugin/redshift/RedshiftIdentifierProcessorTest.java | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java index 453c8e52db..fb59504f67 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java @@ -53,13 +53,13 @@ public String quoteIdentifierIgnoreCase(String identifier) { } /** - * Unconditional quoting for DDL-generation call sites: {@code null} returns - * {@code null}, otherwise wraps with double quotes after stripping one + * Unconditional quoting for DDL-generation call sites: {@code null}/blank pass + * through unchanged, otherwise wraps with double quotes after stripping one * surrounding quote pair and doubling every embedded double quote. */ public String quoteIdentifierAlways(String identifier) { - if (identifier == null) { - return null; + if (StringUtils.isBlank(identifier)) { + return identifier; } return "\"" + escapeIdentifierContent(identifier) + "\""; } diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java index 84f95b00a4..373431f41e 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java @@ -74,7 +74,8 @@ void quoteIdentifierAlwaysWrapsAndEscapes() { assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("foo")); assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("\"foo\"")); assertEquals("\"we\"\"ird\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("we\"ird")); - assertEquals("\"\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("")); + // blank passes through unchanged instead of producing an empty quoted identifier + assertEquals("", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("")); assertNull(RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(null)); } From f236b4f74b5e1efa601df65a2687aafd381fbf56 Mon Sep 17 00:00:00 2001 From: zgq Date: Wed, 29 Jul 2026 18:36:50 +0800 Subject: [PATCH 5/5] fix(redshift): align identifier contract with PostgreSQL Reuse the verified PostgreSQL processor, preserve raw identifier round trips, and handle optional SHOW CREATE TABLE schema qualification. Co-authored-by: HandSonic <8078023+handsonic@users.noreply.github.com> --- .../plugin/redshift/RedshiftMetaData.java | 8 +- .../RedshiftIdentifierProcessor.java | 92 +------------------ .../RedshiftIdentifierProcessorTest.java | 46 +++++++--- 3 files changed, 47 insertions(+), 99 deletions(-) diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java index 6ae56b0872..0b970610af 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/RedshiftMetaData.java @@ -38,8 +38,14 @@ public String tableDDL(Connection connection, String databaseName, String schema } static String buildShowCreateTableSql(String schemaName, String tableName) { - return "SHOW CREATE TABLE " + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(schemaName) + "." + if (tableName == null || tableName.isEmpty()) { + throw new IllegalArgumentException("Redshift table name must not be empty"); + } + String qualifiedName = schemaName == null || schemaName.isEmpty() + ? RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(tableName) + : RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(schemaName) + "." + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(tableName); + return "SHOW CREATE TABLE " + qualifiedName; } @Override diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java index fb59504f67..13d18f343b 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/main/java/ai/chat2db/plugin/redshift/identifier/RedshiftIdentifierProcessor.java @@ -1,95 +1,13 @@ package ai.chat2db.plugin.redshift.identifier; -import ai.chat2db.spi.DefaultSQLIdentifierProcessor; -import org.apache.commons.lang3.StringUtils; +import ai.chat2db.plugin.postgresql.identifier.PostgreSQLIdentifierProcessor; /** - * Redshift dialect identifier processor: double-quoted identifiers with embedded-quote - * doubling, and single-quote doubling for string literals. Shared stateless - * instance available via {@link #INSTANCE} for call sites without MetaData access. - * - *

{@link #quoteIdentifier(String)} follows the SPI contract: it is conditional - * and only quotes when the identifier is not already a valid plain identifier - * (or is a reserved keyword). {@link #quoteIdentifierAlways(String)} is the - * unconditional variant reserved for DDL-generation call sites that historically - * always produced quoted output. + * Redshift follows PostgreSQL identifier folding, quoting, reserved-word, and + * string-literal rules. Keep a dialect-specific instance while reusing the shared + * PostgreSQL contract so completion and generated SQL cannot drift apart. */ -public class RedshiftIdentifierProcessor extends DefaultSQLIdentifierProcessor { +public class RedshiftIdentifierProcessor extends PostgreSQLIdentifierProcessor { public static final RedshiftIdentifierProcessor INSTANCE = new RedshiftIdentifierProcessor(); - - /** - * SPI-facing conditional quoting: returns {@code null} for {@code null}, - * blank input unchanged, a valid plain identifier that is not a reserved - * keyword unquoted, and otherwise wraps with double quotes after stripping - * one surrounding quote pair and doubling every embedded double quote. - */ - @Override - public String quoteIdentifier(String identifier) { - if (StringUtils.isBlank(identifier)) { - return identifier; - } - if (isValidIdentifier(identifier) && !isReservedKeyword(identifier.toUpperCase(), null, null)) { - return identifier; - } - return quoteIdentifierAlways(identifier); - } - - @Override - public String quoteIdentifier(String identifier, Integer majorVersion, Integer minorVersion) { - return quoteIdentifier(identifier); - } - - /** - * SPI "preserve case, always quote" variant: unconditionally wraps with - * double quotes (blank input returned unchanged). - */ - @Override - public String quoteIdentifierIgnoreCase(String identifier) { - if (StringUtils.isBlank(identifier)) { - return identifier; - } - return quoteIdentifierAlways(identifier); - } - - /** - * Unconditional quoting for DDL-generation call sites: {@code null}/blank pass - * through unchanged, otherwise wraps with double quotes after stripping one - * surrounding quote pair and doubling every embedded double quote. - */ - public String quoteIdentifierAlways(String identifier) { - if (StringUtils.isBlank(identifier)) { - return identifier; - } - return "\"" + escapeIdentifierContent(identifier) + "\""; - } - - /** - * Escapes a value interpolated into a single-quoted SQL string literal by - * doubling every single quote. - */ - @Override - public String escapeString(String str) { - return str == null ? null : StringUtils.replace(str, "'", "''"); - } - - private static String escapeIdentifierContent(String identifier) { - String stripped = identifier; - if (stripped.length() >= 2 && stripped.startsWith("\"") && stripped.endsWith("\"")) { - stripped = stripped.substring(1, stripped.length() - 1); - } - return StringUtils.replace(stripped, "\"", "\"\""); - } - - /** - * Escapes identifier content for a position already surrounded by double - * quotes: strips one surrounding quote pair, then doubles every embedded - * double quote. - */ - public static String escapeIdentifier(String identifier) { - if (identifier == null) { - return null; - } - return escapeIdentifierContent(identifier); - } } diff --git a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java index 373431f41e..97faa75408 100644 --- a/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java +++ b/chat2db-community-server/chat2db-community-plugins/chat2db-community-redshift/src/test/java/ai/chat2db/plugin/redshift/RedshiftIdentifierProcessorTest.java @@ -3,8 +3,11 @@ import ai.chat2db.plugin.redshift.identifier.RedshiftIdentifierProcessor; import org.junit.jupiter.api.Test; +import java.util.List; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class RedshiftIdentifierProcessorTest { @@ -25,9 +28,9 @@ void escapeIdentifierDoublesEmbeddedDoubleQuotes() { } @Test - void escapeIdentifierStripsOneSurroundingQuotePair() { - assertEquals("foo", RedshiftIdentifierProcessor.escapeIdentifier("\"foo\"")); - assertEquals("fo\"\"o", RedshiftIdentifierProcessor.escapeIdentifier("\"fo\"o\"")); + void escapeIdentifierTreatsWrappingQuotesAsRawContent() { + assertEquals("\"\"foo\"\"", RedshiftIdentifierProcessor.escapeIdentifier("\"foo\"")); + assertEquals("\"\"fo\"\"o\"\"", RedshiftIdentifierProcessor.escapeIdentifier("\"fo\"o\"")); } @Test @@ -41,11 +44,13 @@ void quoteIdentifierQuotesWhenNotPlain() { assertEquals("\"we\"\"ird\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("we\"ird")); assertEquals("\"has space\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("has space")); assertEquals("\"2leading\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("2leading")); + assertEquals("\"MixedCase\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("MixedCase")); + assertEquals("\"SELECT\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("SELECT")); } @Test void quoteIdentifierRequotesAlreadyQuotedInput() { - assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("\"foo\"")); + assertEquals("\"\"\"foo\"\"\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifier("\"foo\"")); } @Test @@ -62,8 +67,8 @@ void versionedQuoteIdentifierDelegatesToConditional() { } @Test - void quoteIdentifierIgnoreCaseAlwaysQuotes() { - assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase("foo")); + void quoteIdentifierIgnoreCaseUsesPostgreSqlConditionalRules() { + assertEquals("foo", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase("foo")); assertEquals("\"we\"\"ird\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase("we\"ird")); assertEquals("", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase("")); assertNull(RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierIgnoreCase(null)); @@ -72,13 +77,20 @@ void quoteIdentifierIgnoreCaseAlwaysQuotes() { @Test void quoteIdentifierAlwaysWrapsAndEscapes() { assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("foo")); - assertEquals("\"foo\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("\"foo\"")); + assertEquals("\"\"\"foo\"\"\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("\"foo\"")); assertEquals("\"we\"\"ird\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("we\"ird")); - // blank passes through unchanged instead of producing an empty quoted identifier - assertEquals("", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("")); + assertEquals("\"\"", RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways("")); assertNull(RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(null)); } + @Test + void alwaysQuoteRoundTripsEveryRawIdentifierShape() { + for (String raw : List.of("plain", "a\"b", "\"already\"", "\"", "a.b", "", " ")) { + assertEquals(raw, RedshiftIdentifierProcessor.INSTANCE.removeIdentifierQuote( + RedshiftIdentifierProcessor.INSTANCE.quoteIdentifierAlways(raw)), raw); + } + } + @Test void maliciousSchemaNameIsNeutralizedInShowCreateTable() { String sql = RedshiftMetaData.buildShowCreateTableSql("public\"; DROP TABLE users; --", "t"); @@ -96,7 +108,19 @@ void maliciousTableNameIsNeutralizedInShowCreateTable() { void benignNamesProduceSameSqlAsBefore() { assertEquals("SHOW CREATE TABLE \"public\".\"orders\"", RedshiftMetaData.buildShowCreateTableSql("public", "orders")); - assertEquals("SHOW CREATE TABLE \"public\".\"orders\"", - RedshiftMetaData.buildShowCreateTableSql("\"public\"", "\"orders\"")); + } + + @Test + void showCreateTableHandlesOptionalSchemaAndRejectsEmptyTable() { + assertEquals("SHOW CREATE TABLE \"orders\"", + RedshiftMetaData.buildShowCreateTableSql(null, "orders")); + assertEquals("SHOW CREATE TABLE \"orders\"", + RedshiftMetaData.buildShowCreateTableSql("", "orders")); + assertEquals("SHOW CREATE TABLE \" \".\" \"", + RedshiftMetaData.buildShowCreateTableSql(" ", " ")); + assertThrows(IllegalArgumentException.class, + () -> RedshiftMetaData.buildShowCreateTableSql("public", "")); + assertThrows(IllegalArgumentException.class, + () -> RedshiftMetaData.buildShowCreateTableSql("public", null)); } }