From 9aaf6ac55a347d2c1ddcb896f8e0adf9c2d9c36e Mon Sep 17 00:00:00 2001 From: Qi Yu Date: Wed, 21 Oct 2026 21:04:24 +0800 Subject: [PATCH 1/2] [#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975) ### What changes were proposed in this pull request? Column default literals (#13973): - `JdbcColumnDefaultValueConverter`: escape string, date and time default literals through a new overridable `quoteStringLiteral` (backslash escapes plus doubled apostrophes). Numeric defaults must parse as a number, or the request fails with `Invalid numeric column default value`. - PostgreSQL and Hologres: override `quoteStringLiteral` to double apostrophes only, and turn `''` back into `'` when parsing a default read from the database. - Doris CREATE TABLE: render string defaults in the form the Doris version stores correctly. 2.1 and later keep the literal text and parse it again as a double-quoted literal, so the value is double-quoted with backslash escapes. 1.2 decodes the literal at CREATE TABLE, so it keeps the single-quoted form. The version is looked up only when a string default contains `'`, `"` or `\`. Table properties (#13974): - Doris: escape property keys and values in ALTER TABLE `SetProperty`, as CREATE TABLE already does. - StarRocks: escape property keys and values on CREATE TABLE, CREATE DATABASE and ALTER TABLE `SetProperty`. Unescape properties parsed from `SHOW CREATE TABLE`, as Doris does. ### Why are the changes needed? Column defaults and table properties went into the generated DDL unescaped. A value containing a quote produced invalid DDL, and a value carrying quotes and SQL text changed the statement. On StarRocks the property key is affected too: any key with the `dynamic_partition.` or `binlog.` prefix passes the alterable-property check. Fix: #13973 Fix: #13974 ### Does this PR introduce _any_ user-facing change? Yes. String defaults and property values with quotes or backslashes are now stored as given. A non-numeric value for a numeric column default is rejected instead of being written into the DDL. ### How was this patch tested? - Unit tests for the common, PostgreSQL, Hologres and Doris default converters, Doris CREATE TABLE SQL generation, Doris and StarRocks `SetProperty` SQL generation, and StarRocks `generatePropertiesSql` / `extractPropertiesFromSql`. - New Docker cases that create a table, add a column and update a default with values such as `O'Brien`, `a""b`, `\path` and a trailing backslash, then check the loaded default (and, for Doris, the stored row): | Class | Result | |---|---| | `TestMysqlTableOperations` | 18 passed | | `TestPostgreSqlTableOperations` | 12 passed | | `TestOceanBaseTableOperations` | 19 passed | | `TestDorisTableOperations` (1.2) | 13 passed | | `CatalogDoris2xIT` (2.1.0, `-PdorisMultiVersionTest`) | 6 passed | The double-quoted Doris form was also checked on 3.0.6.2 and 4.0.6 (`CatalogDoris3xIT` / `CatalogDoris4xIT` passed) before the 1.2 path was made version-aware. `TestStarRocksTableOperations`, the version-aware rerun on 3.0 and 4.0, and the Docker ITs for the property changes were not run locally. --------- Co-authored-by: yuqi1129 # Conflicts: # catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java # catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java # catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java # catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java --- .../HologresColumnDefaultValueConverter.java | 15 +- ...stHologresColumnDefaultValueConverter.java | 27 ++ .../TestOceanBaseTableOperations.java | 54 +++ .../JdbcColumnDefaultValueConverter.java | 33 +- .../TestJdbcColumnDefaultValueConverter.java | 45 ++ .../DorisColumnDefaultValueConverter.java | 81 ++++ .../operation/DorisDatabaseOperations.java | 17 +- .../doris/operation/DorisTableOperations.java | 122 +++++- .../catalog/doris/utils/DorisUtils.java | 8 +- .../TestDorisColumnDefaultValueConverter.java | 53 +++ .../integration/test/CatalogDoris2xIT.java | 44 ++ .../integration/test/CatalogDoris3xIT.java | 45 ++ .../integration/test/CatalogDoris4xIT.java | 45 ++ .../TestDorisDatabaseOperations.java | 19 + ...TestDorisDatabasePropertiesOperations.java | 96 +++++ .../operation/TestDorisTableOperations.java | 51 +++ ...TestDorisTableOperationsSqlGeneration.java | 393 ++++++++++++++++++ .../catalog/doris/utils/TestDorisUtils.java | 35 ++ .../operation/TestMysqlTableOperations.java | 56 +++ ...PostgreSqlColumnDefaultValueConverter.java | 15 +- ...PostgreSqlColumnDefaultValueConverter.java | 65 +++ .../integration/test/CatalogPostgreSqlIT.java | 2 +- .../TestPostgreSqlTableOperations.java | 56 +++ .../operations/StarRocksTableOperations.java | 4 +- .../starrocks/utils/StarRocksUtils.java | 22 +- .../TestStarRocksTableOperations.java | 55 +++ ...StarRocksTableOperationsSqlGeneration.java | 14 + .../starrocks/utils/TestStarRocksUtils.java | 54 +++ 28 files changed, 1497 insertions(+), 29 deletions(-) create mode 100644 catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabasePropertiesOperations.java create mode 100644 catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlColumnDefaultValueConverter.java diff --git a/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/converter/HologresColumnDefaultValueConverter.java b/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/converter/HologresColumnDefaultValueConverter.java index 4cb166582fc..da3168b8b28 100644 --- a/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/converter/HologresColumnDefaultValueConverter.java +++ b/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/converter/HologresColumnDefaultValueConverter.java @@ -46,7 +46,8 @@ public HologresColumnDefaultValueConverter() {} // for example: // '2021-10-01 00:00:00'::timestamp without time zone // ''something'::character varying' - private static final Pattern VALUE_WITH_TYPE = Pattern.compile("'(.+?)'::\\s*(.+)"); + // Possessive repetition avoids recursive backtracking for long default literals. + private static final Pattern VALUE_WITH_TYPE = Pattern.compile("'((?:[^']|'')*+)'::\\s*(.+)"); // for example: // NULL::character varying @@ -86,10 +87,20 @@ public Expression toGravitino( } } + /** + * Doubles single quotes only. With {@code standard_conforming_strings} on, the default since + * PostgreSQL 9.1, a backslash in a string literal is an ordinary character. + */ + @Override + protected String quoteStringLiteral(String value) { + return "'" + value.replace("'", "''") + "'"; + } + private Expression parseLiteral(JdbcTypeConverter.JdbcTypeBean type, String columnDefaultValue) { Matcher matcher = VALUE_WITH_TYPE.matcher(columnDefaultValue); if (matcher.find()) { - columnDefaultValue = matcher.group(1); + // String literals in the catalog are shown with embedded single quotes doubled. + columnDefaultValue = matcher.group(1).replace("''", "'"); } if (columnDefaultValue.startsWith(NULL_PREFIX)) { diff --git a/catalogs-contrib/catalog-jdbc-hologres/src/test/java/org/apache/gravitino/catalog/hologres/converter/TestHologresColumnDefaultValueConverter.java b/catalogs-contrib/catalog-jdbc-hologres/src/test/java/org/apache/gravitino/catalog/hologres/converter/TestHologresColumnDefaultValueConverter.java index cd38893a586..c5d18d6998b 100644 --- a/catalogs-contrib/catalog-jdbc-hologres/src/test/java/org/apache/gravitino/catalog/hologres/converter/TestHologresColumnDefaultValueConverter.java +++ b/catalogs-contrib/catalog-jdbc-hologres/src/test/java/org/apache/gravitino/catalog/hologres/converter/TestHologresColumnDefaultValueConverter.java @@ -34,6 +34,17 @@ public class TestHologresColumnDefaultValueConverter { private final HologresColumnDefaultValueConverter converter = new HologresColumnDefaultValueConverter(); + /** Verifies long quoted text defaults can be loaded without recursive regex stack growth. */ + @Test + public void testLongQuotedTextLiteralIsUnescaped() { + String value = "text '::quoted \\path ".repeat(2000); + JdbcTypeConverter.JdbcTypeBean type = new JdbcTypeConverter.JdbcTypeBean("text"); + Assertions.assertEquals( + Literals.stringLiteral(value), + converter.toGravitino( + type, converter.fromGravitino(Literals.stringLiteral(value)) + "::text", false, true)); + } + @Test public void testNullDefaultValueNullable() { JdbcTypeConverter.JdbcTypeBean type = new JdbcTypeConverter.JdbcTypeBean("int4"); @@ -206,4 +217,20 @@ public void testFromGravitinoDefaultValueNotSet() { String result = converter.fromGravitino(DEFAULT_VALUE_NOT_SET); Assertions.assertNull(result); } + + @Test + public void testStringLiteralDoublesQuotesOnly() { + Assertions.assertEquals( + "'O''Brien \\path'", converter.fromGravitino(Literals.stringLiteral("O'Brien \\path"))); + } + + @Test + public void testQuotedTextLiteralIsUnescaped() { + JdbcTypeConverter.JdbcTypeBean type = new JdbcTypeConverter.JdbcTypeBean("text"); + Assertions.assertEquals( + Literals.stringLiteral("O'Brien'::x \\path"), + converter.toGravitino(type, "'O''Brien''::x \\path'::text", false, true)); + Assertions.assertEquals( + Literals.stringLiteral(""), converter.toGravitino(type, "''::text", false, true)); + } } diff --git a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java index a0d4b2f9c97..9d08cb61f84 100644 --- a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java +++ b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java @@ -1262,4 +1262,58 @@ public void testAlterTableSkipsEmptySql() { JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); Assertions.assertEquals("Updated comment", table.columns()[0].comment()); } + + @ParameterizedTest + @ValueSource(strings = {"O'Brien", "quote\\' and \\new\\path; \"double\"", "ends with \\"}) + void testStringDefaultValueRoundTrips(String defaultValue) { + String tableName = RandomNameUtils.genRandomName("oceanbase_default_round_trip"); + JdbcColumn column = + JdbcColumn.builder() + .withName("col1") + .withType(VARCHAR) + .withNullable(true) + .withDefaultValue(Literals.varcharLiteral(255, defaultValue)) + .build(); + TABLE_OPERATIONS.create( + TEST_DB_NAME, + tableName, + new JdbcColumn[] {column}, + null, + Collections.emptyMap(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + Indexes.EMPTY_INDEXES); + try { + JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, defaultValue), table.columns()[0].defaultValue()); + + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME, + tableName, + TableChange.addColumn( + new String[] {"col2"}, + VARCHAR, + null, + TableChange.ColumnPosition.defaultPos(), + true, + false, + Literals.varcharLiteral(255, defaultValue))); + table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, defaultValue), table.columns()[1].defaultValue()); + + String updated = defaultValue + " updated"; + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME, + tableName, + TableChange.updateColumnDefaultValue( + new String[] {"col1"}, Literals.varcharLiteral(255, updated))); + table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, updated), table.columns()[0].defaultValue()); + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME, tableName); + } + } } diff --git a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/converter/JdbcColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/converter/JdbcColumnDefaultValueConverter.java index f2fa7d85b0d..ac3099a4015 100644 --- a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/converter/JdbcColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/converter/JdbcColumnDefaultValueConverter.java @@ -18,8 +18,10 @@ */ package org.apache.gravitino.catalog.jdbc.converter; +import static org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.escapeSqlLiteral; import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; +import java.math.BigDecimal; import java.time.LocalDateTime; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatterBuilder; @@ -77,6 +79,8 @@ public String fromGravitino(Expression defaultValue) { // case value is an empty string, we should wrap it with single quotes to avoid SQL error. if (StringUtils.isBlank(value)) { value = "'%s'".formatted(value); + } else { + value = validateNumericLiteral(value); } return value; } else if (type instanceof Types.TimestampType) { @@ -84,11 +88,11 @@ public String fromGravitino(Expression defaultValue) { * @see LocalDateTime#toString() would return like 'yyyy-MM-ddTHH:mm:ss' */ if (literal.value() instanceof String || literal.value() instanceof LocalDateTime) { - return String.format("'%s'", literal.value().toString().replace("T", " ")); + return quoteStringLiteral(literal.value().toString().replace("T", " ")); } - return String.format("'%s'", literal.value()); + return quoteStringLiteral(String.valueOf(literal.value())); } else { - return String.format("'%s'", literal.value()); + return quoteStringLiteral(String.valueOf(literal.value())); } } @@ -102,4 +106,27 @@ public Expression toGravitino( boolean nullable) { return DEFAULT_VALUE_NOT_SET; } + + /** + * Renders a value as a quoted SQL string literal for a column default. The default escapes + * backslashes and doubles single quotes, as MySQL-compatible dialects require. Dialects with + * different string literal rules override this method. + * + * @param value the unquoted literal value + * @return the quoted and escaped SQL string literal + */ + protected String quoteStringLiteral(String value) { + return "'" + escapeSqlLiteral(value, '\'') + "'"; + } + + private static String validateNumericLiteral(String value) { + String trimmed = value.trim(); + try { + new BigDecimal(trimmed); + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + String.format("Invalid numeric column default value: %s", value), e); + } + return trimmed; + } } diff --git a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/converter/TestJdbcColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/converter/TestJdbcColumnDefaultValueConverter.java index 7d9cf881cda..14070fafe2b 100644 --- a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/converter/TestJdbcColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/converter/TestJdbcColumnDefaultValueConverter.java @@ -23,8 +23,11 @@ import org.apache.gravitino.rel.Column; import org.apache.gravitino.rel.expressions.Expression; import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.types.Types; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class TestJdbcColumnDefaultValueConverter { @@ -58,4 +61,46 @@ public void testFromGravitino() { result = converter.fromGravitino(expression); Assertions.assertEquals("1234", result); } + + @Test + public void testStringLiteralIsEscaped() { + Assertions.assertEquals( + "'O''Brien'", converter.fromGravitino(Literals.stringLiteral("O'Brien"))); + Assertions.assertEquals( + "'a\\\\'' , 1); DROP TABLE t; --'", + converter.fromGravitino(Literals.varcharLiteral(64, "a\\' , 1); DROP TABLE t; --"))); + Assertions.assertEquals( + "'it''s'", converter.fromGravitino(Literals.of("it's", Types.FixedCharType.of(8)))); + Assertions.assertEquals( + "'2024-01-01 00:00:00'''", + converter.fromGravitino( + Literals.of("2024-01-01T00:00:00'", Types.TimestampType.withoutTimeZone()))); + Assertions.assertEquals( + "'2024-01-01'''", + converter.fromGravitino(Literals.of("2024-01-01'", Types.DateType.get()))); + } + + @ParameterizedTest + @ValueSource(strings = {"0", "-12", "+3", "1.50", " 42 ", "1.0E10", "-2.5e-3"}) + public void testValidNumericLiteralIsWritten(String value) { + Assertions.assertEquals( + value.trim(), converter.fromGravitino(Literals.of(value, Types.DecimalType.of(20, 4)))); + } + + @ParameterizedTest + @ValueSource(strings = {"1); DROP TABLE t; --", "1 OR 1=1", "abc", "NaN", "0x10", "1,000"}) + public void testInvalidNumericLiteralIsRejected(String value) { + IllegalArgumentException exception = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> converter.fromGravitino(Literals.of(value, Types.IntegerType.get()))); + Assertions.assertEquals( + "Invalid numeric column default value: " + value, exception.getMessage()); + } + + @Test + public void testBlankNumericLiteralKeepsQuotedEmptyString() { + Assertions.assertEquals( + "''", converter.fromGravitino(Literals.of("", Types.IntegerType.get()))); + } } diff --git a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java index 0d0c8d033d3..09eb63ec144 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java @@ -88,6 +88,87 @@ public String fromGravitinoForAddColumn( return super.fromGravitino(defaultValue); } +<<<<<<< HEAD +======= + /** + * Converts a CREATE TABLE default value to Doris SQL. + * + *

Doris 2.1 and later keep the text of a string default unchanged when the table is created + * and parse it again as a double-quoted literal when a row takes the default, so string defaults + * must use that form there. Earlier versions decode the literal when the table is created and use + * the shared single-quoted form. + * + * @param defaultValue the Gravitino default value + * @param useDoubleQuotedLiteral whether string literals use double quotes with backslash escapes + * @return the Doris SQL representation, or {@code null} when the default is unset + */ + @Nullable + public String fromGravitinoForCreateTable( + Expression defaultValue, boolean useDoubleQuotedLiteral) { + if (useDoubleQuotedLiteral && isNonNullStringLiteral(defaultValue)) { + return quoteDorisLiteral(String.valueOf(((Literal) defaultValue).value()), false, false); + } + return super.fromGravitino(defaultValue); + } + + /** + * Returns whether a default value is a non-null string, varchar or char literal. + * + * @param defaultValue the Gravitino default value + * @return whether the value is a non-null string literal + */ + public static boolean isNonNullStringLiteral(Expression defaultValue) { + if (!(defaultValue instanceof Literal)) { + return false; + } + Literal literal = (Literal) defaultValue; + return literal.value() != null + && (literal.dataType() instanceof Types.StringType + || literal.dataType() instanceof Types.VarCharType + || literal.dataType() instanceof Types.FixedCharType); + } + + /** + * Converts a loaded default value for a Doris MODIFY COLUMN definition. + * + *

String literals containing quote or backslash characters need Doris-specific escaping. + * Unparsed expressions come from native Doris metadata and are passed through unchanged. + * + * @param defaultValue the loaded Gravitino default value + * @param doubleEscapeBackslashes whether Doris 3.x requires an additional backslash-escaping + * layer + * @param tripleEscapeQuotes whether Doris 3.x MODIFY COLUMN requires three backslashes before an + * embedded double quote + * @return the Doris SQL representation, or {@code null} when the default is unset + */ + @Nullable + public String fromGravitinoForModifyColumn( + Expression defaultValue, boolean doubleEscapeBackslashes, boolean tripleEscapeQuotes) { + if (DEFAULT_VALUE_NOT_SET.equals(defaultValue)) { + return null; + } + + if (defaultValue instanceof UnparsedExpression) { + return ((UnparsedExpression) defaultValue).unparsedExpression(); + } + + if (defaultValue instanceof Literal) { + Literal literal = (Literal) defaultValue; + if (literal.value() != null + && (literal.dataType() instanceof Types.StringType + || literal.dataType() instanceof Types.VarCharType + || literal.dataType() instanceof Types.FixedCharType)) { + String value = String.valueOf(literal.value()); + if (value.indexOf('\\') >= 0 || value.indexOf('\'') >= 0 || value.indexOf('"') >= 0) { + return quoteDorisModifyLiteral(value, doubleEscapeBackslashes, tripleEscapeQuotes); + } + } + } + + return super.fromGravitino(defaultValue); + } + +>>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) @Override public Expression toGravitino( JdbcTypeConverter.JdbcTypeBean columnType, diff --git a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisDatabaseOperations.java b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisDatabaseOperations.java index 1f0a37c1538..ff5e8ddae51 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisDatabaseOperations.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisDatabaseOperations.java @@ -28,6 +28,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.commons.lang3.StringUtils; import org.apache.gravitino.catalog.doris.utils.DorisUtils; import org.apache.gravitino.catalog.jdbc.JdbcSchema; @@ -40,6 +42,9 @@ public class DorisDatabaseOperations extends JdbcDatabaseOperations { public static final String COMMENT_KEY = "comment"; + private static final Pattern DATABASE_PROPERTY_PATTERN = + Pattern.compile("\"(.*?)\"\\s*=\\s*\"(.*)\"\\s*,?"); + @Override public String generateCreateDatabaseSql( String databaseName, String comment, Map properties) { @@ -130,7 +135,17 @@ protected Map getDatabaseProperties(String databaseName) { throw new NoSuchTableException("Database %s does not exist.", databaseName); } - return DorisUtils.extractPropertiesFromSql(createDatabaseSql); + // SHOW CREATE DATABASE uses PrintableMap to wrap raw metadata in quotes without escaping it. + // Capture through the last quote on each property line and preserve the contents verbatim; + // SQL-literal unescaping would corrupt doubled quotes and backslashes. + Map properties = new HashMap<>(); + for (String line : createDatabaseSql.split("\n")) { + Matcher matcher = DATABASE_PROPERTY_PATTERN.matcher(line.trim()); + if (matcher.matches()) { + properties.put(matcher.group(1), matcher.group(2)); + } + } + return properties; } @Override diff --git a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java index 0f2876948d4..323bc0a0477 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java @@ -164,6 +164,7 @@ protected String generateCreateTableSql( // Doris auto-increment requires 2.1.0+. Validate before delegating to base class. validateAutoIncrementVersion(columns); + boolean useDoubleQuotedStringDefaults = useDoubleQuotedCreateTableStringDefaults(columns); validateIncrementCol(columns, indexes); validateDistribution(distribution, columns); @@ -183,7 +184,7 @@ protected String generateCreateTableSql( .append(BACK_QUOTE) .append(column.name()) .append(BACK_QUOTE); - appendColumnDefinition(column, columnsSql); + appendColumnDefinition(column, columnsSql, useDoubleQuotedStringDefaults); return columnsSql.toString(); }) .collect(Collectors.joining(",\n"))); @@ -328,6 +329,27 @@ private void validateAutoIncrementVersion(JdbcColumn[] columns) { } } + /** + * Returns whether CREATE TABLE string defaults must use Doris double-quoted literals. The Doris + * version is looked up only when a string default contains a quote or backslash, since plain + * strings render the same in both forms. + */ + private boolean useDoubleQuotedCreateTableStringDefaults(JdbcColumn[] columns) { + boolean needsVersion = + Arrays.stream(columns) + .map(Column::defaultValue) + .filter(DorisColumnDefaultValueConverter::isNonNullStringLiteral) + .map(defaultValue -> String.valueOf(((Literal) defaultValue).value())) + .anyMatch( + value -> + value.indexOf('\'') >= 0 + || value.indexOf('"') >= 0 + || value.indexOf('\\') >= 0); + return needsVersion + && isVersionAtLeast( + getDorisVersion("CREATE TABLE default literal compatibility check"), 2, 1, 0); + } + private String getDorisVersion(String purpose) { Preconditions.checkState(dataSource != null, "dataSource is required for version validation"); String version = null; @@ -846,7 +868,15 @@ protected String generateAlterTableSql( } else if (change instanceof TableChange.UpdateColumnType) { lazyLoadTable = getOrCreateTable(databaseName, tableName, lazyLoadTable); TableChange.UpdateColumnType updateColumnType = (TableChange.UpdateColumnType) change; +<<<<<<< HEAD alterSql.add(updateColumnTypeFieldDefinition(updateColumnType, lazyLoadTable)); +======= + alterColumnDorisVersion = + resolveModifyColumnVersion(updateColumnType, lazyLoadTable, alterColumnDorisVersion); + alterSql.add( + updateColumnTypeFieldDefinition( + updateColumnType, lazyLoadTable, alterColumnDorisVersion)); +>>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) } else if (change instanceof TableChange.UpdateColumnComment) { TableChange.UpdateColumnComment updateColumnComment = (TableChange.UpdateColumnComment) change; @@ -855,7 +885,12 @@ protected String generateAlterTableSql( lazyLoadTable = getOrCreateTable(databaseName, tableName, lazyLoadTable); TableChange.UpdateColumnPosition updateColumnPosition = (TableChange.UpdateColumnPosition) change; - alterSql.add(updateColumnPositionFieldDefinition(updateColumnPosition, lazyLoadTable)); + alterColumnDorisVersion = + resolveModifyColumnVersion( + updateColumnPosition, lazyLoadTable, alterColumnDorisVersion); + alterSql.add( + updateColumnPositionFieldDefinition( + updateColumnPosition, lazyLoadTable, alterColumnDorisVersion)); } else if (change instanceof TableChange.DeleteColumn) { TableChange.DeleteColumn deleteColumn = (TableChange.DeleteColumn) change; lazyLoadTable = getOrCreateTable(databaseName, tableName, lazyLoadTable); @@ -865,9 +900,14 @@ protected String generateAlterTableSql( } } else if (change instanceof TableChange.UpdateColumnNullability) { lazyLoadTable = getOrCreateTable(databaseName, tableName, lazyLoadTable); + TableChange.UpdateColumnNullability updateColumnNullability = + (TableChange.UpdateColumnNullability) change; + alterColumnDorisVersion = + resolveModifyColumnVersion( + updateColumnNullability, lazyLoadTable, alterColumnDorisVersion); alterSql.add( updateColumnNullabilityDefinition( - (TableChange.UpdateColumnNullability) change, lazyLoadTable)); + updateColumnNullability, lazyLoadTable, alterColumnDorisVersion)); } else if (change instanceof TableChange.AddIndex) { alterSql.add(addIndexDefinition((TableChange.AddIndex) change)); } else if (change instanceof TableChange.DeleteIndex) { @@ -905,8 +945,48 @@ protected String generateAlterTableSql( return result; } +<<<<<<< HEAD +======= + private static void validateIndexChangeConflicts(TableChange... changes) { + Set deleteIndexNames = new HashSet<>(); + Set addIndexNames = new HashSet<>(); + + for (TableChange change : changes) { + if (change instanceof TableChange.DeleteIndex) { + String indexName = ((TableChange.DeleteIndex) change).getName(); + Preconditions.checkArgument( + deleteIndexNames.add(indexName), + "Index '%s' cannot be deleted more than once in the same request", + indexName); + Preconditions.checkArgument( + !addIndexNames.contains(indexName), + "Index '%s' cannot be added and deleted in the same request", + indexName); + } else if (change instanceof TableChange.AddIndex) { + String indexName = ((TableChange.AddIndex) change).getName(); + Preconditions.checkArgument( + !deleteIndexNames.contains(indexName), + "Index '%s' cannot be added and deleted in the same request", + indexName); + addIndexNames.add(indexName); + } + } + } + + private Optional resolveModifyColumnVersion( + TableChange.ColumnChange change, JdbcTable table, Optional dorisVersion) { + if (dorisVersion.isPresent() || change.fieldName().length != 1) { + return dorisVersion; + } + JdbcColumn column = getJdbcColumnFromTable(table, change.fieldName()[0]); + return requiresVersionAwareModifyEscaping(column) + ? Optional.of(getDorisVersion("MODIFY COLUMN default literal compatibility check")) + : dorisVersion; + } + +>>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) private String updateColumnNullabilityDefinition( - TableChange.UpdateColumnNullability change, JdbcTable table) { + TableChange.UpdateColumnNullability change, JdbcTable table, Optional dorisVersion) { validateUpdateColumnNullable(change, table); String col = change.fieldName()[0]; JdbcColumn column = getJdbcColumnFromTable(table, col); @@ -923,7 +1003,7 @@ private String updateColumnNullabilityDefinition( + BACK_QUOTE + col + BACK_QUOTE - + appendColumnDefinition(updateColumn, new StringBuilder()); + + appendColumnDefinitionForModify(updateColumn, new StringBuilder(), dorisVersion); } private String generateTableProperties(List setProperties) { @@ -932,7 +1012,9 @@ private String generateTableProperties(List setProperti .map( setProperty -> String.format( - "\"%s\" = \"%s\"", setProperty.getProperty(), setProperty.getValue())) + "\"%s\" = \"%s\"", + escapeSqlLiteral(setProperty.getProperty(), '"'), + escapeSqlLiteral(setProperty.getValue(), '"'))) .collect(Collectors.joining(",\n")); return "set (" + properties + ")"; } @@ -1045,7 +1127,9 @@ private String serializeAddColumnDefaultValue( } private String updateColumnPositionFieldDefinition( - TableChange.UpdateColumnPosition updateColumnPosition, JdbcTable jdbcTable) { + TableChange.UpdateColumnPosition updateColumnPosition, + JdbcTable jdbcTable, + Optional dorisVersion) { if (updateColumnPosition.fieldName().length > 1) { throw new UnsupportedOperationException("Doris does not support nested column names."); } @@ -1053,7 +1137,7 @@ private String updateColumnPositionFieldDefinition( JdbcColumn column = getJdbcColumnFromTable(jdbcTable, col); StringBuilder columnDefinition = new StringBuilder(); columnDefinition.append("MODIFY COLUMN ").append(BACK_QUOTE).append(col).append(BACK_QUOTE); - appendColumnDefinition(column, columnDefinition); + appendColumnDefinitionForModify(column, columnDefinition, dorisVersion); if (updateColumnPosition.getPosition() instanceof TableChange.First) { columnDefinition.append("FIRST"); } else if (updateColumnPosition.getPosition() instanceof TableChange.After) { @@ -1114,7 +1198,8 @@ private String updateColumnTypeFieldDefinition( return appendColumnDefinition(newColumn, sqlBuilder).toString(); } - private StringBuilder appendColumnDefinition(JdbcColumn column, StringBuilder sqlBuilder) { + private StringBuilder appendColumnDefinition( + JdbcColumn column, StringBuilder sqlBuilder, boolean useDoubleQuotedStringDefault) { // Add data type sqlBuilder.append(SPACE).append(typeConverter.fromGravitino(column.dataType())).append(SPACE); @@ -1126,7 +1211,11 @@ private StringBuilder appendColumnDefinition(JdbcColumn column, StringBuilder sq } // Add DEFAULT value if specified - appendDefaultValue(column, sqlBuilder); + if (useDoubleQuotedStringDefault) { + appendCreateTableDefaultValue(column, sqlBuilder); + } else { + appendDefaultValue(column, sqlBuilder); + } // Add column auto_increment if specified if (column.autoIncrement()) { @@ -1140,6 +1229,19 @@ private StringBuilder appendColumnDefinition(JdbcColumn column, StringBuilder sq return sqlBuilder; } + private void appendCreateTableDefaultValue(JdbcColumn column, StringBuilder sqlBuilder) { + if (DEFAULT_VALUE_NOT_SET.equals(column.defaultValue())) { + return; + } + Preconditions.checkState( + columnDefaultValueConverter instanceof DorisColumnDefaultValueConverter, + "DorisColumnDefaultValueConverter is required for Doris CREATE TABLE"); + String defaultValue = + ((DorisColumnDefaultValueConverter) columnDefaultValueConverter) + .fromGravitinoForCreateTable(column.defaultValue(), true); + sqlBuilder.append("DEFAULT ").append(defaultValue).append(SPACE); + } + static String addIndexDefinition(TableChange.AddIndex addIndex) { // PRIMARY_KEY and UNIQUE_KEY are table-level concepts in Doris, not index-level // They should not be added via ALTER TABLE ADD INDEX diff --git a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/utils/DorisUtils.java b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/utils/DorisUtils.java index c0a437e9662..658ba58e0c0 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/utils/DorisUtils.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/utils/DorisUtils.java @@ -85,7 +85,9 @@ public static Map extractPropertiesFromSql(String createTableSql String[] lines = createTableSql.split("\n"); boolean isProperties = false; - final String sProperties = "\"(.*)\"\\s*=\\s*\"(.*)\",?"; + // Escape alternatives exclude the closing delimiter, so no backtracking is needed. + final String sProperties = + "\"((?:\\\\.|\"\"|[^\"\\\\])*+)\"\\s*=\\s*\"((?:\\\\.|\"\"|[^\"\\\\])*+)\",?"; final Pattern patternProperties = Pattern.compile(sProperties); for (String line : lines) { @@ -98,8 +100,8 @@ public static Map extractPropertiesFromSql(String createTableSql if (matcherProperties.find()) { // generatePropertiesSql escapes keys and values for double-quoted literals, and SHOW // CREATE echoes them escaped; unescape both so callers observe the original text. - final String key = unescapeSqlLiteral(matcherProperties.group(1).trim(), '"'); - String value = unescapeSqlLiteral(matcherProperties.group(2).trim(), '"'); + final String key = unescapeSqlLiteral(matcherProperties.group(1), '"'); + String value = unescapeSqlLiteral(matcherProperties.group(2), '"'); properties.put(key, value); } } diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java index 0f494cbc797..cf87e64fcd8 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java @@ -138,4 +138,57 @@ public void testUnescapeStringLikeDefaultValues() { Literals.of("owner's a\"\"b \"value\"\\path", Types.VarCharType.of(255)), CONVERTER.toGravitino(varcharType, "owner's a\"\"b \"value\"\\\\path", false, false)); } +<<<<<<< HEAD +======= + + /** Verifies loaded literal and native expression defaults use MODIFY COLUMN SQL forms. */ + @Test + public void testModifyColumnSerializesLoadedDefaultValues() { + Assertions.assertNull( + CONVERTER.fromGravitinoForModifyColumn(DEFAULT_VALUE_NOT_SET, false, false)); + Assertions.assertEquals( + "NULL", CONVERTER.fromGravitinoForModifyColumn(Literals.NULL, false, false)); + Assertions.assertEquals( + "7", CONVERTER.fromGravitinoForModifyColumn(Literals.integerLiteral(7), false, false)); + Assertions.assertEquals( + "CURRENT_TIMESTAMP", + CONVERTER.fromGravitinoForModifyColumn(DEFAULT_VALUE_OF_CURRENT_TIMESTAMP, false, false)); + Assertions.assertEquals( + "CURRENT_DATE", + CONVERTER.fromGravitinoForModifyColumn( + UnparsedExpression.of("CURRENT_DATE"), false, false)); + + Assertions.assertEquals( + "\"owner's \\\"value\\\"\\\\path\"", + CONVERTER.fromGravitinoForModifyColumn( + Literals.of("owner's \"value\"\\path", Types.VarCharType.of(255)), false, false)); + } + + /** Verifies adjacent backslash and quote escaping for Doris 3.x and 4.x. */ + @Test + public void testModifyColumnEscapesAdjacentBackslashAndQuoteByDorisVersion() { + String defaultValue = "prefix" + "\\" + "\"" + "suffix"; + + Assertions.assertEquals( + "\"" + "prefix" + "\\".repeat(3) + "\"suffix\"", + CONVERTER.fromGravitinoForModifyColumn( + Literals.of(defaultValue, Types.VarCharType.of(255)), false, false)); + Assertions.assertEquals( + "\"" + "prefix" + "\\".repeat(7) + "\"suffix\"", + CONVERTER.fromGravitinoForModifyColumn( + Literals.of(defaultValue, Types.VarCharType.of(255)), true, true)); + } + + @Test + public void testCreateTableStringDefaultLiteralForms() { + DorisColumnDefaultValueConverter converter = new DorisColumnDefaultValueConverter(); + Literal value = Literals.varcharLiteral(64, "O'Brien C:\\tmp \"x\""); + Assertions.assertEquals( + "\"O'Brien C:\\\\tmp \\\"x\\\"\"", converter.fromGravitinoForCreateTable(value, true)); + Assertions.assertEquals( + "'O''Brien C:\\\\tmp \"x\"'", converter.fromGravitinoForCreateTable(value, false)); + Assertions.assertEquals( + "12", converter.fromGravitinoForCreateTable(Literals.integerLiteral(12), true)); + } +>>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) } diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris2xIT.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris2xIT.java index c810bc6fab8..a008dd0dd31 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris2xIT.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris2xIT.java @@ -45,6 +45,7 @@ import org.apache.gravitino.rel.TableCatalog; import org.apache.gravitino.rel.expressions.NamedReference; import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.literals.Literals; import org.apache.gravitino.rel.expressions.transforms.Transforms; import org.apache.gravitino.rel.types.Types; import org.junit.jupiter.api.AfterAll; @@ -139,6 +140,49 @@ void testTableCommentWithNereids(String comment) throws Exception { } } + @ParameterizedTest + @ValueSource( + strings = { + "O'Brien", + "quote\\' and \\new\\path; \"double\"", + "owner's a\"\"b \"value\"\\path", + "ends with \\" + }) + void testCreateTableStringDefaultIsStoredVerbatim(String defaultValue) throws Exception { + TableCatalog tables = catalog.asTableCatalog(); + NameIdentifier identifier = + NameIdentifier.of(schemaName, GravitinoITUtils.genRandomName("create_default")); + tables.createTable( + identifier, + new Column[] { + Column.of("id", Types.IntegerType.get(), null, false, false, null), + Column.of( + "col1", + Types.VarCharType.of(255), + null, + true, + false, + Literals.varcharLiteral(255, defaultValue)) + }, + null, + Collections.emptyMap(), + Transforms.EMPTY_TRANSFORM, + Distributions.hash(1, NamedReference.field("id")), + null); + + try (Connection connection = + DriverManager.getConnection( + jdbcUrl + schemaName, DorisContainer.USER_NAME, DorisContainer.PASSWORD); + Statement statement = connection.createStatement()) { + statement.executeUpdate(String.format("INSERT INTO `%s` (id) VALUES (1)", identifier.name())); + try (ResultSet result = + statement.executeQuery(String.format("SELECT col1 FROM `%s`", identifier.name()))) { + assertTrue(result.next()); + assertEquals(defaultValue, result.getString(1)); + } + } + } + private void createMetalake() { client.createMetalake(metalakeName, "comment", Collections.emptyMap()); metalake = client.loadMetalake(metalakeName); diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris3xIT.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris3xIT.java index cea2800808a..eff2fbb17fd 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris3xIT.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris3xIT.java @@ -71,6 +71,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Integration tests for Doris 3.0.x specific features: INVERTED index syntax, Auto Increment, and @@ -118,6 +120,49 @@ public void resetSchema() { createSchema(); } + @ParameterizedTest + @ValueSource( + strings = { + "O'Brien", + "quote\\' and \\new\\path; \"double\"", + "owner's a\"\"b \"value\"\\path", + "ends with \\" + }) + void testCreateTableStringDefaultIsStoredVerbatim(String defaultValue) throws Exception { + TableCatalog tables = catalog.asTableCatalog(); + NameIdentifier identifier = + NameIdentifier.of(schemaName, GravitinoITUtils.genRandomName("create_default")); + tables.createTable( + identifier, + new Column[] { + Column.of("id", Types.IntegerType.get(), null, false, false, null), + Column.of( + "col1", + Types.VarCharType.of(255), + null, + true, + false, + Literals.varcharLiteral(255, defaultValue)) + }, + null, + Collections.emptyMap(), + Transforms.EMPTY_TRANSFORM, + Distributions.hash(1, NamedReference.field("id")), + null); + + try (Connection connection = + DriverManager.getConnection( + jdbcUrl + schemaName, DorisContainer.USER_NAME, DorisContainer.PASSWORD); + Statement statement = connection.createStatement()) { + statement.executeUpdate(String.format("INSERT INTO `%s` (id) VALUES (1)", identifier.name())); + try (ResultSet result = + statement.executeQuery(String.format("SELECT col1 FROM `%s`", identifier.name()))) { + assertTrue(result.next()); + assertEquals(defaultValue, result.getString(1)); + } + } + } + private void createMetalake() { client.createMetalake(metalakeName, "comment", Collections.emptyMap()); metalake = client.loadMetalake(metalakeName); diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java index 948f8bace4b..b6c8c4fd4a9 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java @@ -72,6 +72,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Integration tests for Doris 4.0.x specific features: BITMAP→INVERTED mapping, ANN/VECTOR index, @@ -120,6 +122,49 @@ public void resetSchema() { createSchema(); } + @ParameterizedTest + @ValueSource( + strings = { + "O'Brien", + "quote\\' and \\new\\path; \"double\"", + "owner's a\"\"b \"value\"\\path", + "ends with \\" + }) + void testCreateTableStringDefaultIsStoredVerbatim(String defaultValue) throws Exception { + TableCatalog tables = catalog.asTableCatalog(); + NameIdentifier identifier = + NameIdentifier.of(schemaName, GravitinoITUtils.genRandomName("create_default")); + tables.createTable( + identifier, + new Column[] { + Column.of("id", Types.IntegerType.get(), null, false, false, null), + Column.of( + "col1", + Types.VarCharType.of(255), + null, + true, + false, + Literals.varcharLiteral(255, defaultValue)) + }, + null, + Collections.emptyMap(), + Transforms.EMPTY_TRANSFORM, + Distributions.hash(1, NamedReference.field("id")), + null); + + try (Connection connection = + DriverManager.getConnection( + jdbcUrl + schemaName, DorisContainer.USER_NAME, DorisContainer.PASSWORD); + Statement statement = connection.createStatement()) { + statement.executeUpdate(String.format("INSERT INTO `%s` (id) VALUES (1)", identifier.name())); + try (ResultSet result = + statement.executeQuery(String.format("SELECT col1 FROM `%s`", identifier.name()))) { + assertTrue(result.next()); + assertEquals(defaultValue, result.getString(1)); + } + } + } + private void createMetalake() { client.createMetalake(metalakeName, "comment", Collections.emptyMap()); metalake = client.loadMetalake(metalakeName); diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseOperations.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseOperations.java index 67134528167..8f48c1df00f 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseOperations.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseOperations.java @@ -26,6 +26,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @Tag("gravitino-docker-test") public class TestDorisDatabaseOperations extends TestDoris { @@ -48,6 +50,23 @@ public void testBaseOperationDatabase() { testDropDatabase(databaseName); } + @ParameterizedTest + @ValueSource( + strings = { + "a\" = \"b", + "a\"\"b", + "two\\\\backslashes and trailing\\", + " leading and trailing whitespace " + }) + void testRawDatabasePropertiesRoundTrip(String value) { + String databaseName = RandomNameUtils.genRandomName("it_db_raw"); + try { + testBaseOperation(databaseName, Map.of("property1", value), value); + } finally { + DATABASE_OPERATIONS.delete(databaseName, true); + } + } + @Test void testListSystemDatabase() { List databaseNames = DATABASE_OPERATIONS.listDatabases(); diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabasePropertiesOperations.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabasePropertiesOperations.java new file mode 100644 index 00000000000..2608804787c --- /dev/null +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabasePropertiesOperations.java @@ -0,0 +1,96 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.doris.operation; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.sql.DataSource; +import org.apache.gravitino.catalog.doris.converter.DorisExceptionConverter; +import org.apache.gravitino.catalog.jdbc.JdbcSchema; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +class TestDorisDatabasePropertiesOperations { + + @ParameterizedTest(name = "{index}") + @MethodSource("rawPropertyValues") + void testLoadPreservesRawDatabaseProperties(String value) throws Exception { + Map properties = new HashMap<>(); + properties.put("comment", value); + properties.put("property1", value); + properties.put("key\"with\"quotes", value); + // Doris SHOW CREATE DATABASE uses PrintableMap, which quotes raw metadata without escaping it. + String showCreateDatabase = + "CREATE DATABASE `test_database`\nPROPERTIES (\n" + + properties.entrySet().stream() + .map(entry -> "\"" + entry.getKey() + "\" = \"" + entry.getValue() + "\"") + .collect(Collectors.joining(",\n")) + + "\n)"; + DataSource dataSource = mock(DataSource.class); + Connection connection = mock(Connection.class); + DatabaseMetaData metadata = mock(DatabaseMetaData.class); + ResultSet catalogs = mock(ResultSet.class); + PreparedStatement statement = mock(PreparedStatement.class); + ResultSet result = mock(ResultSet.class); + when(dataSource.getConnection()).thenReturn(connection); + when(connection.getMetaData()).thenReturn(metadata); + when(metadata.getCatalogs()).thenReturn(catalogs); + when(catalogs.next()).thenReturn(true, false); + when(catalogs.getString("TABLE_CAT")).thenReturn("test_database"); + when(connection.prepareStatement("SHOW CREATE DATABASE `test_database`")).thenReturn(statement); + when(statement.executeQuery()).thenReturn(result); + when(result.next()).thenReturn(true, false); + when(result.getString("Create Database")).thenReturn(showCreateDatabase); + DorisDatabaseOperations operations = new DorisDatabaseOperations(); + operations.initialize(dataSource, new DorisExceptionConverter(), Collections.emptyMap()); + + JdbcSchema schema = operations.load("test_database"); + + assertEquals("test_database", schema.name()); + assertEquals(value, schema.comment()); + properties.remove("comment"); + assertEquals(properties, schema.properties()); + verify(result).close(); + verify(statement).close(); + } + + private static Stream rawPropertyValues() { + return Stream.of( + "owner's \"comment\" C:\\tmp; DROP DATABASE marker; --", + "a\" = \"b", + "a\"\"b", + "two\\\\backslashes and trailing\\", + "\\\"quoted\\\"", + " leading and trailing whitespace ", + "", + "long\" = \"value\\\\".repeat(2000)); + } +} diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java index a503eb3c5eb..20ddc08631a 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java @@ -22,6 +22,10 @@ import static org.junit.Assert.assertTrue; import com.google.common.collect.Maps; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; import java.time.LocalDate; import java.util.ArrayList; import java.util.Arrays; @@ -60,6 +64,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @Tag("gravitino-docker-test") public class TestDorisTableOperations extends TestDoris { @@ -767,4 +773,49 @@ public boolean isMySQLDriverVersionSupported(String driverVersion) { operationsWithOldDriver.calculateDatetimePrecision("DATETIME(3)", 0, 0), "DATETIME(3) should return 3 even with unsupported driver version"); } + + @ParameterizedTest + @ValueSource( + strings = { + "O'Brien", + "quote\\' and \\new\\path; \"double\"", + "owner's a\"\"b \"value\"\\path", + "ends with \\" + }) + void testCreateTableStringDefaultIsStoredVerbatim(String defaultValue) throws SQLException { + String tableName = GravitinoITUtils.genRandomName("doris_default_round_trip"); + JdbcColumn[] columns = { + JdbcColumn.builder().withName("id").withType(INT).withNullable(false).build(), + JdbcColumn.builder() + .withName("col1") + .withType(VARCHAR_255) + .withNullable(true) + .withDefaultValue(Literals.varcharLiteral(255, defaultValue)) + .build() + }; + TABLE_OPERATIONS.create( + databaseName, + tableName, + columns, + null, + createProperties(), + Transforms.EMPTY_TRANSFORM, + Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("id")), + Indexes.EMPTY_INDEXES); + try { + try (Connection connection = DATA_SOURCE.getConnection(); + Statement statement = connection.createStatement()) { + statement.executeUpdate( + String.format("INSERT INTO `%s`.`%s` (id) VALUES (1)", databaseName, tableName)); + try (ResultSet result = + statement.executeQuery( + String.format("SELECT col1 FROM `%s`.`%s`", databaseName, tableName))) { + Assertions.assertTrue(result.next()); + Assertions.assertEquals(defaultValue, result.getString(1)); + } + } + } finally { + TABLE_OPERATIONS.drop(databaseName, tableName); + } + } } diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java index 3aa83591c0d..298fc00ff0a 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java @@ -39,6 +39,7 @@ import org.apache.gravitino.catalog.jdbc.JdbcTable; import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter; import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.Expression; import org.apache.gravitino.rel.expressions.NamedReference; import org.apache.gravitino.rel.expressions.distributions.Distribution; import org.apache.gravitino.rel.expressions.distributions.Distributions; @@ -118,6 +119,303 @@ public String createTableSqlWithIndexes( distribution, indexes); } +<<<<<<< HEAD +======= + + List indexes(Connection connection, String databaseName, String tableName) + throws SQLException { + return getIndexes(connection, databaseName, tableName); + } + } + + /** Verifies string defaults and unchanged column attributes are preserved. */ + @Test + public void testUpdateColumnTypePreservesStringDefaultAndColumnAttributes() { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + JdbcColumn column = + JdbcColumn.builder() + .withName("col1") + .withType(Types.VarCharType.of(10)) + .withComment("comment") + .withNullable(true) + .withDefaultValue(Literals.of("seed", Types.VarCharType.of(10))) + .build(); + + Assertions.assertEquals( + "ALTER TABLE `test_table`\n" + + "MODIFY COLUMN `col1` varchar(20) NULL DEFAULT 'seed' COMMENT 'comment' ;", + ops.updateColumnTypeSql(column, Types.VarCharType.of(20))); + } + + /** Verifies numeric and current-timestamp defaults are preserved. */ + @Test + public void testUpdateColumnTypePreservesNumericAndCurrentTimestampDefaults() { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + JdbcColumn numericColumn = + JdbcColumn.builder() + .withName("col1") + .withType(Types.IntegerType.get()) + .withNullable(false) + .withDefaultValue(Literals.integerLiteral(7)) + .build(); + JdbcColumn timestampColumn = + JdbcColumn.builder() + .withName("col1") + .withType(Types.TimestampType.withoutTimeZone()) + .withNullable(false) + .withDefaultValue(DEFAULT_VALUE_OF_CURRENT_TIMESTAMP) + .build(); + + Assertions.assertEquals( + "ALTER TABLE `test_table`\nMODIFY COLUMN `col1` bigint NOT NULL DEFAULT 7 ;", + ops.updateColumnTypeSql(numericColumn, Types.LongType.get())); + Assertions.assertEquals( + "ALTER TABLE `test_table`\n" + + "MODIFY COLUMN `col1` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP ;", + ops.updateColumnTypeSql(timestampColumn, Types.TimestampType.withoutTimeZone(6))); + } + + /** Verifies quoted string defaults use Doris MODIFY COLUMN escaping. */ + @Test + public void testUpdateColumnTypeUsesDorisModifyEscapingForQuotedDefaults() { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + Literal defaultValue = Literals.of("owner's \"value\"\\path", Types.VarCharType.of(32)); + JdbcColumn column = + JdbcColumn.builder() + .withName("col1") + .withType(Types.VarCharType.of(32)) + .withNullable(false) + .withDefaultValue(defaultValue) + .build(); + + String sql = ops.updateColumnTypeSql(column, Types.VarCharType.of(64)); + + Assertions.assertTrue( + sql.contains( + "DEFAULT " + + new DorisColumnDefaultValueConverter() + .fromGravitinoForModifyColumn(defaultValue, true, true)), + sql); + } + + /** + * Verifies nullability and position changes use the same default encoding as MODIFY COLUMN. + * + * @param version the Doris version + */ + @ParameterizedTest + @ValueSource(strings = {"1.2.7", "2.1.0", "3.0.6.2", "4.0.6"}) + public void testNullabilityAndPositionPreserveEscapedDefaults(String version) throws Exception { + for (String value : new String[] {"O'Brien", "a\"\"b", "\\path", "trailing\\"}) { + TestableDorisTableOperations ops = new TestableDorisTableOperations(version); + DataSource versionDataSource = mockVersionDataSource(version); + ops.setDataSource(versionDataSource); + JdbcColumn column = + JdbcColumn.builder() + .withName("col1") + .withType(Types.VarCharType.of(64)) + .withNullable(true) + .withComment("column comment") + .withDefaultValue(Literals.varcharLiteral(64, value)) + .build(); + ops.setTableForAlter( + JdbcTable.builder() + .withName("test_table") + .withColumns(new JdbcColumn[] {column}) + .build()); + + String sql = + ops.alterTableSql( + "test_table", + TableChange.updateColumnNullability(new String[] {"col1"}, false), + TableChange.updateColumnPosition( + new String[] {"col1"}, TableChange.ColumnPosition.first()), + TableChange.updateColumnType(new String[] {"col1"}, Types.VarCharType.of(128))); + + boolean isDoris3x = version.startsWith("3."); + String expectedDefault = + new DorisColumnDefaultValueConverter() + .fromGravitinoForModifyColumn(column.defaultValue(), isDoris3x, isDoris3x); + Assertions.assertEquals( + "ALTER TABLE `test_table`\n" + + "MODIFY COLUMN `col1` varchar(64) NOT NULL DEFAULT " + + expectedDefault + + " COMMENT 'column comment' ,\n" + + "MODIFY COLUMN `col1` varchar(64) NULL DEFAULT " + + expectedDefault + + " COMMENT 'column comment' FIRST,\n" + + "MODIFY COLUMN `col1` varchar(128) NULL DEFAULT " + + expectedDefault + + " COMMENT 'column comment' ;", + sql); + if (value.contains("\\") || value.contains("\"")) { + Mockito.verify(versionDataSource, Mockito.times(1)).getConnection(); + } else { + Mockito.verifyNoInteractions(versionDataSource); + } + } + } + + /** Verifies ordinary defaults do not add a version query for nullability or position changes. */ + @Test + public void testNullabilityAndPositionSkipVersionLookupForPlainDefaults() { + for (Expression defaultValue : + new Expression[] {DEFAULT_VALUE_NOT_SET, Literals.NULL, Literals.integerLiteral(7)}) { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + DataSource versionDataSource = Mockito.mock(DataSource.class); + ops.setDataSource(versionDataSource); + JdbcColumn column = + JdbcColumn.builder() + .withName("col1") + .withType(Types.IntegerType.get()) + .withDefaultValue(defaultValue) + .build(); + ops.setTableForAlter( + JdbcTable.builder() + .withName("test_table") + .withColumns(new JdbcColumn[] {column}) + .build()); + String sql = + ops.alterTableSql( + "test_table", + TableChange.updateColumnNullability(new String[] {"col1"}, true), + TableChange.updateColumnPosition( + new String[] {"col1"}, TableChange.ColumnPosition.first())); + String expectedDefault = + DEFAULT_VALUE_NOT_SET.equals(defaultValue) + ? "" + : "DEFAULT " + (Literals.NULL.equals(defaultValue) ? "NULL" : "7") + " "; + Assertions.assertTrue( + sql.contains( + "int NULL " + + expectedDefault + + ",\nMODIFY COLUMN `col1` int NULL " + + expectedDefault + + "FIRST;"), + sql); + Mockito.verifyNoInteractions(versionDataSource); + } + } + + /** Verifies native unparsed defaults pass through without interpretation. */ + @Test + public void testUpdateColumnTypePassesThroughUnparsedDefault() { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + JdbcColumn column = + JdbcColumn.builder() + .withName("col1") + .withType(Types.DateType.get()) + .withNullable(false) + .withDefaultValue(UnparsedExpression.of("CURRENT_DATE")) + .build(); + + Assertions.assertTrue( + ops.updateColumnTypeSql(column, Types.DateType.get()).contains("DEFAULT CURRENT_DATE"), + "Native default expression should pass through unchanged"); + } + + /** Verifies explicit NULL and an unset default remain distinct in generated SQL. */ + @Test + public void testUpdateColumnTypePreservesNullAndUnsetDefaultDistinction() { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + JdbcColumn nullDefault = + JdbcColumn.builder() + .withName("col1") + .withType(Types.IntegerType.get()) + .withNullable(true) + .withDefaultValue(Literals.NULL) + .build(); + JdbcColumn unsetDefault = + JdbcColumn.builder() + .withName("col1") + .withType(Types.IntegerType.get()) + .withNullable(true) + .withDefaultValue(DEFAULT_VALUE_NOT_SET) + .build(); + + Assertions.assertTrue( + ops.updateColumnTypeSql(nullDefault, Types.LongType.get()).contains("NULL DEFAULT NULL ;")); + Assertions.assertTrue( + ops.updateColumnTypeSql(unsetDefault, Types.LongType.get()).contains("bigint NULL ;")); + } + + /** + * Verifies rejected nested-column changes follow the alter-table exception contract. + * + * @param change the nested-column change to reject + */ + @ParameterizedTest + @MethodSource("nestedColumnChanges") + public void testRejectsNestedColumnName(TableChange change) { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + + IllegalArgumentException exception = + Assertions.assertThrows( + IllegalArgumentException.class, () -> ops.alterTableSql("test_table", change)); + + Assertions.assertEquals("Doris does not support nested column names.", exception.getMessage()); + } + + /** Verifies version lookup is cached across type changes in one ALTER request. */ + @Test + public void testUpdateColumnTypeQueriesVersionOnceForAlterRequest() throws Exception { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + DataSource versionDataSource = mockVersionDataSource("doris-3.0.6.2-rc01-910c4249c5"); + ops.setDataSource(versionDataSource); + JdbcColumn firstColumn = + JdbcColumn.builder() + .withName("col1") + .withType(Types.VarCharType.of(32)) + .withDefaultValue(Literals.of("first\\value", Types.VarCharType.of(32))) + .build(); + JdbcColumn secondColumn = + JdbcColumn.builder() + .withName("col2") + .withType(Types.VarCharType.of(32)) + .withDefaultValue(Literals.of("second\"value", Types.VarCharType.of(32))) + .build(); + ops.setTableForAlter( + JdbcTable.builder() + .withName("test_table") + .withColumns(new JdbcColumn[] {firstColumn, secondColumn}) + .build()); + + ops.alterTableSql( + "test_table", + TableChange.updateColumnType(new String[] {"col1"}, Types.VarCharType.of(64)), + TableChange.updateColumnType(new String[] {"col2"}, Types.VarCharType.of(64))); + + Mockito.verify(versionDataSource, Mockito.times(1)).getConnection(); + } + + /** Verifies ADD and MODIFY changes share one version lookup in an ALTER request. */ + @Test + public void testAddAndUpdateColumnTypeShareVersionLookup() throws Exception { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + DataSource versionDataSource = mockVersionDataSource("doris-3.0.6.2-rc01-910c4249c5"); + ops.setDataSource(versionDataSource); + JdbcColumn existingColumn = + JdbcColumn.builder() + .withName("col1") + .withType(Types.VarCharType.of(32)) + .withDefaultValue(Literals.of("existing\\value", Types.VarCharType.of(32))) + .build(); + ops.setTableForAlter( + JdbcTable.builder() + .withName("test_table") + .withColumns(new JdbcColumn[] {existingColumn}) + .build()); + + ops.alterTableSql( + "test_table", + TableChange.addColumn( + new String[] {"col2"}, + Types.VarCharType.of(32), + Literals.of("added\\value", Types.VarCharType.of(32))), + TableChange.updateColumnType(new String[] {"col1"}, Types.VarCharType.of(64))); + + Mockito.verify(versionDataSource, Mockito.times(1)).getConnection(); +>>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) } /** @@ -960,6 +1258,73 @@ private static DataSource mockBackendDataSource(int aliveBackendCount) throws Ex return dataSource; } + @Test + public void testCreateTableStringDefaultUsesVersionLiteralForm() throws Exception { + String singleQuoted = "'O''Brien \\\\ \"x\"'"; + String doubleQuoted = "\"O'Brien \\\\ \\\"x\\\"\""; + Map expectedDefaults = new LinkedHashMap<>(); + expectedDefaults.put("doris-1.2.7-1-Unknown", singleQuoted); + expectedDefaults.put("doris-2.1.0-rc11-1", doubleQuoted); + expectedDefaults.put("doris-3.0.6.2-rc01-910c4249c5", doubleQuoted); + expectedDefaults.put("doris-4.0.6-release-a851eab4", doubleQuoted); + for (Map.Entry entry : expectedDefaults.entrySet()) { + String version = entry.getKey(); + String expectedDefault = entry.getValue(); + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + DataSource versionDataSource = mockVersionDataSource(version); + ops.setDataSource(versionDataSource); + JdbcColumn[] columns = { + JdbcColumn.builder().withName("id").withType(Types.IntegerType.get()).build(), + JdbcColumn.builder() + .withName("owner") + .withType(Types.VarCharType.of(64)) + .withDefaultValue(Literals.varcharLiteral(64, "O'Brien \\ \"x\"")) + .build() + }; + + String sql = + ops.generateCreateTableSql( + "test_table", + columns, + null, + Collections.singletonMap(REPLICATION_FACTOR, "1"), + Transforms.EMPTY_TRANSFORM, + Distributions.hash(1, NamedReference.field("id")), + Indexes.EMPTY_INDEXES); + + Assertions.assertTrue(sql.contains("DEFAULT " + expectedDefault + " "), sql); + Mockito.verify(versionDataSource, Mockito.times(1)).getConnection(); + } + } + + @Test + public void testCreateTablePlainStringDefaultSkipsVersionLookup() throws Exception { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + DataSource versionDataSource = Mockito.mock(DataSource.class); + ops.setDataSource(versionDataSource); + JdbcColumn[] columns = { + JdbcColumn.builder().withName("id").withType(Types.IntegerType.get()).build(), + JdbcColumn.builder() + .withName("owner") + .withType(Types.VarCharType.of(64)) + .withDefaultValue(Literals.varcharLiteral(64, "plain")) + .build() + }; + + String sql = + ops.generateCreateTableSql( + "test_table", + columns, + null, + Collections.singletonMap(REPLICATION_FACTOR, "1"), + Transforms.EMPTY_TRANSFORM, + Distributions.hash(1, NamedReference.field("id")), + Indexes.EMPTY_INDEXES); + + Assertions.assertTrue(sql.contains("DEFAULT 'plain' "), sql); + Mockito.verifyNoInteractions(versionDataSource); + } + private static DataSource mockVersionDataSource(String dorisVersion) throws Exception { // FRONTENDS() exposes the Doris version instead of the MySQL protocol version. DataSource dataSource = Mockito.mock(DataSource.class); @@ -973,4 +1338,32 @@ private static DataSource mockVersionDataSource(String dorisVersion) throws Exce Mockito.when(resultSet.getString(1)).thenReturn(dorisVersion); return dataSource; } +<<<<<<< HEAD +======= + + private static Stream nestedColumnChanges() { + String[] fieldName = {"parent", "child"}; + return Stream.of( + TableChange.addColumn(fieldName, Types.LongType.get()), + TableChange.updateColumnComment(fieldName, "new comment"), + TableChange.updateColumnPosition(fieldName, TableChange.ColumnPosition.first()), + TableChange.deleteColumn(fieldName, false), + TableChange.updateColumnType(fieldName, Types.LongType.get())); + } + + @Test + public void testSetPropertyEscapesKeyAndValue() { + TestableDorisTableOperations ops = new TestableDorisTableOperations(); + String sql = + ops.alterTableSql( + "test_table", + TableChange.setProperty("key\"name", "owner's \"comment\" C:\\tmp\"); --"), + TableChange.setProperty("plain", "value")); + Assertions.assertEquals( + "ALTER TABLE `test_table`\n" + + "set (\"key\"\"name\" = \"owner's \"\"comment\"\" C:\\\\tmp\"\"); --\",\n" + + "\"plain\" = \"value\");", + sql); + } +>>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) } diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/utils/TestDorisUtils.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/utils/TestDorisUtils.java index 017254fa362..45e804cbffd 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/utils/TestDorisUtils.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/utils/TestDorisUtils.java @@ -74,6 +74,41 @@ public void testGeneratePropertiesSql() { assertEquals(expectedStr, result); } + /** Verifies large escaped properties are parsed without recursive regex stack growth. */ + @Test + public void testLongEscapedPropertiesRoundTrip() { + Map properties = new HashMap<>(); + properties.put("key".repeat(10000), "value\" = \"part\\".repeat(2000)); + assertEquals( + properties, + DorisUtils.extractPropertiesFromSql(DorisUtils.generatePropertiesSql(properties))); + } + + /** Verifies escaped quotes cannot be mistaken for the property key/value separator. */ + @Test + public void testPropertiesWithQuotedSeparatorsRoundTrip() { + Map properties = new HashMap<>(); + properties.put("k", "a\" = \"b"); + properties.put("key\" = \"part", "value\" = \"part\\"); + properties.put(" spaced key ", " spaced value "); + properties.put("empty", ""); + properties.put("\\\"", "\\\" = \"\\"); + + assertEquals( + properties, + DorisUtils.extractPropertiesFromSql(DorisUtils.generatePropertiesSql(properties))); + } + + /** Verifies backslash-escaped quotes around an equals sign remain in the property value. */ + @Test + public void testPropertiesWithBackslashEscapedSeparators() { + String sql = "PROPERTIES (\n\"k\" = \"a\\\" = \\\"b\",\n\"end\" = \"tail\\\\\"\n)"; + Map expected = new HashMap<>(); + expected.put("k", "a\" = \"b"); + expected.put("end", "tail\\"); + assertEquals(expected, DorisUtils.extractPropertiesFromSql(sql)); + } + @Test public void testExtractTablePropertiesFromSql() { // Test when properties is null diff --git a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java index f9b38d6419b..4fe9f4b8bf6 100644 --- a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java +++ b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java @@ -51,6 +51,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @Tag("gravitino-docker-test") public class TestMysqlTableOperations extends TestMysql { @@ -1256,4 +1258,58 @@ public boolean isMySQLDriverVersionSupported(String driverVersion) { operationsWithOldDriver.calculateDatetimePrecision("TIME", 16, 0), "TIME type should return null for unsupported driver version"); } + + @ParameterizedTest + @ValueSource(strings = {"O'Brien", "quote\\' and \\new\\path; \"double\"", "ends with \\"}) + void testStringDefaultValueRoundTrips(String defaultValue) { + String tableName = RandomNameUtils.genRandomName("mysql_default_round_trip"); + JdbcColumn column = + JdbcColumn.builder() + .withName("col1") + .withType(VARCHAR) + .withNullable(true) + .withDefaultValue(Literals.varcharLiteral(255, defaultValue)) + .build(); + TABLE_OPERATIONS.create( + TEST_DB_NAME.toString(), + tableName, + new JdbcColumn[] {column}, + null, + Collections.emptyMap(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + Indexes.EMPTY_INDEXES); + try { + JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, defaultValue), table.columns()[0].defaultValue()); + + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME.toString(), + tableName, + TableChange.addColumn( + new String[] {"col2"}, + VARCHAR, + null, + TableChange.ColumnPosition.defaultPos(), + true, + false, + Literals.varcharLiteral(255, defaultValue))); + table = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, defaultValue), table.columns()[1].defaultValue()); + + String updated = defaultValue + " updated"; + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME.toString(), + tableName, + TableChange.updateColumnDefaultValue( + new String[] {"col1"}, Literals.varcharLiteral(255, updated))); + table = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, updated), table.columns()[0].defaultValue()); + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME.toString(), tableName); + } + } } diff --git a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlColumnDefaultValueConverter.java index 0b4229fcf8b..65e16256a4a 100644 --- a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlColumnDefaultValueConverter.java @@ -40,7 +40,8 @@ public PostgreSqlColumnDefaultValueConverter() {} // for example: // '2021-10-01 00:00:00'::timestamp without time zone // ''something'::character varying' - private static final Pattern VALUE_WITH_TYPE = Pattern.compile("'(.+?)'::\\s*(.+)"); + // Possessive repetition avoids recursive backtracking for long default literals. + private static final Pattern VALUE_WITH_TYPE = Pattern.compile("'((?:[^']|'')*+)'::\\s*(.+)"); // for example: // NULL::character varying @@ -72,10 +73,20 @@ public Expression toGravitino( } } + /** + * Doubles single quotes only. With {@code standard_conforming_strings} on, the default since + * PostgreSQL 9.1, a backslash in a string literal is an ordinary character. + */ + @Override + protected String quoteStringLiteral(String value) { + return "'" + value.replace("'", "''") + "'"; + } + private Expression parseLiteral(JdbcTypeConverter.JdbcTypeBean type, String columnDefaultValue) { Matcher matcher = VALUE_WITH_TYPE.matcher(columnDefaultValue); if (matcher.find()) { - columnDefaultValue = matcher.group(1); + // String literals in the catalog are shown with embedded single quotes doubled. + columnDefaultValue = matcher.group(1).replace("''", "'"); } if (columnDefaultValue.startsWith(NULL_PREFIX)) { diff --git a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlColumnDefaultValueConverter.java new file mode 100644 index 00000000000..c66b17fa35e --- /dev/null +++ b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlColumnDefaultValueConverter.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.postgresql.converter; + +import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class TestPostgreSqlColumnDefaultValueConverter { + + private final PostgreSqlColumnDefaultValueConverter converter = + new PostgreSqlColumnDefaultValueConverter(); + + /** Verifies long quoted text defaults can be loaded without recursive regex stack growth. */ + @Test + public void testLongQuotedTextLiteralIsUnescaped() { + String value = "text '::quoted \\path ".repeat(2000); + JdbcTypeConverter.JdbcTypeBean type = new JdbcTypeConverter.JdbcTypeBean("text"); + Assertions.assertEquals( + Literals.stringLiteral(value), + converter.toGravitino( + type, converter.fromGravitino(Literals.stringLiteral(value)) + "::text", false, true)); + } + + @Test + public void testStringLiteralDoublesQuotesOnly() { + Assertions.assertEquals( + "'O''Brien \\path'", converter.fromGravitino(Literals.stringLiteral("O'Brien \\path"))); + } + + @Test + public void testQuotedTextLiteralIsUnescaped() { + JdbcTypeConverter.JdbcTypeBean type = new JdbcTypeConverter.JdbcTypeBean("text"); + Assertions.assertEquals( + Literals.stringLiteral("O'Brien'::x \\path"), + converter.toGravitino(type, "'O''Brien''::x \\path'::text", false, true)); + Assertions.assertEquals( + Literals.stringLiteral(""), converter.toGravitino(type, "''::text", false, true)); + } + + @Test + void testEmptyBpcharLiteralIsUnescaped() { + JdbcTypeConverter.JdbcTypeBean type = + new JdbcTypeConverter.JdbcTypeBean(PostgreSqlTypeConverter.BPCHAR); + Assertions.assertEquals( + Literals.stringLiteral(""), converter.toGravitino(type, "''::bpchar", false, true)); + } +} diff --git a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java index ec331a1ae82..414d198db4b 100644 --- a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java +++ b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java @@ -1132,7 +1132,7 @@ void testColumnDefaultValueConverter() { Assertions.assertEquals(Column.DEFAULT_VALUE_NOT_SET, column.defaultValue()); break; case "character_col_3": - Assertions.assertEquals(Literals.stringLiteral("''::bpchar"), column.defaultValue()); + Assertions.assertEquals(Literals.stringLiteral(""), column.defaultValue()); break; default: Assertions.fail("Unexpected column name: " + column.name()); diff --git a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java index cef450d4dcb..ef2b1efc9ad 100644 --- a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java +++ b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java @@ -49,6 +49,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @Tag("gravitino-docker-test") public class TestPostgreSqlTableOperations extends TestPostgreSql { @@ -780,4 +782,58 @@ public void testCalculateDatetimePrecision() { TABLE_OPERATIONS.calculateDatetimePrecision("VARCHAR", 50, 0), "Non-datetime type should return 0 precision"); } + + @ParameterizedTest + @ValueSource(strings = {"O'Brien", "quote\\' and \\new\\path; \"double\"", "ends with \\"}) + void testStringDefaultValueRoundTrips(String defaultValue) { + String tableName = RandomNameUtils.genRandomName("pg_default_round_trip"); + JdbcColumn column = + JdbcColumn.builder() + .withName("col1") + .withType(VARCHAR) + .withNullable(true) + .withDefaultValue(Literals.varcharLiteral(255, defaultValue)) + .build(); + TABLE_OPERATIONS.create( + TEST_DB_NAME, + tableName, + new JdbcColumn[] {column}, + null, + Collections.emptyMap(), + Transforms.EMPTY_TRANSFORM, + Distributions.NONE, + Indexes.EMPTY_INDEXES); + try { + JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, defaultValue), table.columns()[0].defaultValue()); + + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME, + tableName, + TableChange.addColumn( + new String[] {"col2"}, + VARCHAR, + null, + TableChange.ColumnPosition.defaultPos(), + true, + false, + Literals.varcharLiteral(255, defaultValue))); + table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, defaultValue), table.columns()[1].defaultValue()); + + String updated = defaultValue + " updated"; + TABLE_OPERATIONS.alterTable( + TEST_DB_NAME, + tableName, + TableChange.updateColumnDefaultValue( + new String[] {"col1"}, Literals.varcharLiteral(255, updated))); + table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, updated), table.columns()[0].defaultValue()); + } finally { + TABLE_OPERATIONS.drop(TEST_DB_NAME, tableName); + } + } } diff --git a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java index c1a9e06bf28..2d978c94b5d 100644 --- a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java +++ b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java @@ -563,7 +563,9 @@ private String renameTableDefinition(TableChange.RenameTable renameTable, JdbcTa private String generateTableProperties(TableChange.SetProperty setProperty) { return String.format( - "set ( \"%s\" = \"%s\" )", setProperty.getProperty(), setProperty.getValue()); + "set ( \"%s\" = \"%s\" )", + escapeSqlLiteral(setProperty.getProperty(), '"'), + escapeSqlLiteral(setProperty.getValue(), '"')); } private String updateColumnPositionFieldDefinition( diff --git a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/utils/StarRocksUtils.java b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/utils/StarRocksUtils.java index 6356e6e22ee..57aa58fdcbe 100644 --- a/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/utils/StarRocksUtils.java +++ b/catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/utils/StarRocksUtils.java @@ -19,6 +19,7 @@ package org.apache.gravitino.catalog.starrocks.utils; import static org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.escapeSqlLiteral; +import static org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.unescapeSqlLiteral; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -32,7 +33,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; -import org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils; import org.apache.gravitino.rel.expressions.NamedReference; import org.apache.gravitino.rel.expressions.distributions.Distribution; import org.apache.gravitino.rel.expressions.distributions.Distributions; @@ -104,7 +104,13 @@ public static String generatePropertiesSql(Map properties) { StringBuilder sqlBuilder = new StringBuilder(" PROPERTIES (\n"); sqlBuilder.append( properties.entrySet().stream() - .map(entry -> "\"" + entry.getKey() + "\"=\"" + entry.getValue() + "\"") + .map( + entry -> + "\"" + + escapeSqlLiteral(entry.getKey(), '"') + + "\"=\"" + + escapeSqlLiteral(entry.getValue(), '"') + + "\"") .collect(Collectors.joining(",\n"))); sqlBuilder.append("\n)"); return sqlBuilder.toString(); @@ -115,7 +121,9 @@ public static Map extractPropertiesFromSql(String createTableSql String[] lines = createTableSql.split("\n"); boolean isProperties = false; - final String sProperties = "\"(.*)\"\\s*=\\s*\"(.*)\",?"; + // Escape alternatives exclude the closing delimiter, so no backtracking is needed. + final String sProperties = + "\"((?:\\\\.|\"\"|[^\"\\\\])*+)\"\\s*=\\s*\"((?:\\\\.|\"\"|[^\"\\\\])*+)\",?"; final Pattern patternProperties = Pattern.compile(sProperties); for (String line : lines) { @@ -126,8 +134,10 @@ public static Map extractPropertiesFromSql(String createTableSql if (isProperties) { final Matcher matcherProperties = patternProperties.matcher(line); if (matcherProperties.find()) { - final String key = matcherProperties.group(1).trim(); - String value = matcherProperties.group(2).trim(); + // generatePropertiesSql escapes keys and values for double-quoted literals, and SHOW + // CREATE echoes them escaped; unescape both so callers observe the original text. + final String key = unescapeSqlLiteral(matcherProperties.group(1), '"'); + String value = unescapeSqlLiteral(matcherProperties.group(2), '"'); properties.put(key, value); } } @@ -201,7 +211,7 @@ public static Distribution extractDistributionInfoFromSql(String createTableSql) public static String extractTableCommentFromSql(String createTableSql) { Matcher matcher = TABLE_COMMENT_PATTERN.matcher(createTableSql.trim()); if (matcher.find()) { - return JdbcConnectorUtils.unescapeSqlLiteral(matcher.group(1), '"'); + return unescapeSqlLiteral(matcher.group(1), '"'); } return ""; } diff --git a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java index 5f531e7d559..934cc425da7 100644 --- a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java +++ b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperations.java @@ -22,6 +22,10 @@ import static org.junit.Assert.assertTrue; import com.google.common.collect.Maps; +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; import java.time.LocalDate; import java.util.ArrayList; import java.util.Arrays; @@ -63,6 +67,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @Tag("gravitino-docker-test") public class TestStarRocksTableOperations extends TestStarRocks { @@ -680,4 +686,53 @@ public void testAlterTableSetPropertyBehavior() { TABLE_OPERATIONS.alterTable( databaseName, tableName, TableChange.setProperty("unsupported.property", "VIEW"))); } + + @ParameterizedTest + @ValueSource( + strings = { + "O'Brien", + "quote\\' and \\new\\path; \"double\"", + "owner's a\"\"b \"value\"\\path", + "ends with \\" + }) + void testStringDefaultValueRoundTrips(String defaultValue) throws SQLException { + String tableName = GravitinoITUtils.genRandomName("starrocks_default_round_trip"); + JdbcColumn[] columns = { + JdbcColumn.builder().withName("id").withType(INT).withNullable(false).build(), + JdbcColumn.builder() + .withName("col1") + .withType(VARCHAR_255) + .withNullable(true) + .withDefaultValue(Literals.varcharLiteral(255, defaultValue)) + .build() + }; + TABLE_OPERATIONS.create( + databaseName, + tableName, + columns, + null, + createProperties(), + Transforms.EMPTY_TRANSFORM, + Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("id")), + Indexes.EMPTY_INDEXES); + try { + JdbcTable table = TABLE_OPERATIONS.load(databaseName, tableName); + Assertions.assertEquals( + Literals.varcharLiteral(255, defaultValue), table.columns()[1].defaultValue()); + try (Connection connection = DATA_SOURCE.getConnection(); + Statement statement = connection.createStatement()) { + statement.executeUpdate( + String.format("INSERT INTO `%s`.`%s` (id) VALUES (1)", databaseName, tableName)); + try (ResultSet result = + statement.executeQuery( + String.format("SELECT col1 FROM `%s`.`%s`", databaseName, tableName))) { + Assertions.assertTrue(result.next()); + Assertions.assertEquals(defaultValue, result.getString(1)); + } + } + + } finally { + TABLE_OPERATIONS.drop(databaseName, tableName); + } + } } diff --git a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperationsSqlGeneration.java b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperationsSqlGeneration.java index b1e32c3f196..d2665ca33c6 100644 --- a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperationsSqlGeneration.java +++ b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/operation/TestStarRocksTableOperationsSqlGeneration.java @@ -296,6 +296,20 @@ public void testEscapeAddColumnCommentInGeneratedSql() { Assertions.assertTrue(alterSql.contains("COMMENT 'owner\\\\''s \"comment\"; --'"), alterSql); } + @Test + void testSetPropertyEscapesKeyAndValue() { + TestableStarRocksTableOperations ops = new TestableStarRocksTableOperations(); + String sql = + ops.alterTableSql( + "test_table", + TableChange.setProperty( + "dynamic_partition.key\"name", "owner's \"comment\" C:\\tmp\"); --")); + Assertions.assertEquals( + "ALTER TABLE `test_table`\n" + + "set ( \"dynamic_partition.key\"\"name\" = \"owner's \"\"comment\"\" C:\\\\tmp\"\"); --\" );", + sql); + } + @Test public void testAddColumnWithNotNullAndDefaultValue() { TestableStarRocksTableOperations ops = new TestableStarRocksTableOperations(); diff --git a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/utils/TestStarRocksUtils.java b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/utils/TestStarRocksUtils.java index 0c023d9014c..e04db6f02cd 100644 --- a/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/utils/TestStarRocksUtils.java +++ b/catalogs/catalog-jdbc-starrocks/src/test/java/org/apache/gravitino/catalog/starrocks/utils/TestStarRocksUtils.java @@ -61,6 +61,11 @@ public void testGeneratePropertiesSql() { result = StarRocksUtils.generatePropertiesSql(properties); assertEquals(" PROPERTIES (\n\"key\"=\"value\"\n)", result); + properties = Collections.singletonMap("key\"name", "owner's \"comment\" C:\\tmp; --"); + result = StarRocksUtils.generatePropertiesSql(properties); + assertEquals( + " PROPERTIES (\n\"key\"\"name\"=\"owner's \"\"comment\"\" C:\\\\tmp; --\"\n)", result); + // Test when properties has multiple entries properties = new HashMap<>(); properties.put("key1", "value1"); @@ -99,6 +104,55 @@ public void testExtractTablePropertiesFromSql() { result = StarRocksUtils.extractPropertiesFromSql(createTableSql); assertEquals("value1", result.get("property1")); assertEquals("comment", result.get("comment")); + + // Escaped SHOW CREATE output (doubled quotes and backslashes, as generatePropertiesSql + // emits) round-trips back to the original key/value text. + createTableSql = + "CREATE DATABASE `test`\nPROPERTIES (\n" + + "\"key\"\"name\" = \"owner's \"\"comment\"\" D:\\\\data; --\"\n)"; + result = StarRocksUtils.extractPropertiesFromSql(createTableSql); + assertEquals("owner's \"comment\" D:\\data; --", result.get("key\"name")); + + // Backslash-style escaping (\" and \\) is unescaped too. + createTableSql = + "CREATE DATABASE `test`\nPROPERTIES (\n" + "\"plain\" = \"owner\\\\s \\\"comment\\\"\"\n)"; + result = StarRocksUtils.extractPropertiesFromSql(createTableSql); + assertEquals("owner\\s \"comment\"", result.get("plain")); + } + + /** Verifies large escaped properties are parsed without recursive regex stack growth. */ + @Test + public void testLongEscapedPropertiesRoundTrip() { + Map properties = new HashMap<>(); + properties.put("key".repeat(10000), "value\" = \"part\\".repeat(2000)); + assertEquals( + properties, + StarRocksUtils.extractPropertiesFromSql(StarRocksUtils.generatePropertiesSql(properties))); + } + + /** Verifies escaped quotes cannot be mistaken for the property key/value separator. */ + @Test + public void testPropertiesWithQuotedSeparatorsRoundTrip() { + Map properties = new HashMap<>(); + properties.put("k", "a\" = \"b"); + properties.put("key\" = \"part", "value\" = \"part\\"); + properties.put(" spaced key ", " spaced value "); + properties.put("empty", ""); + properties.put("\\\"", "\\\" = \"\\"); + + assertEquals( + properties, + StarRocksUtils.extractPropertiesFromSql(StarRocksUtils.generatePropertiesSql(properties))); + } + + /** Verifies backslash-escaped quotes around an equals sign remain in the property value. */ + @Test + public void testPropertiesWithBackslashEscapedSeparators() { + String sql = "PROPERTIES (\n\"k\" = \"a\\\" = \\\"b\",\n\"end\" = \"tail\\\\\"\n)"; + Map expected = new HashMap<>(); + expected.put("k", "a\" = \"b"); + expected.put("end", "tail\\"); + assertEquals(expected, StarRocksUtils.extractPropertiesFromSql(sql)); } @Test From c720a3b14c23a31045db98abcee4a967a3505856 Mon Sep 17 00:00:00 2001 From: yuqi Date: Thu, 22 Oct 2026 07:34:40 +0800 Subject: [PATCH 2/2] fix(catalog-doris): Resolve branch-1.3 cherry-pick conflicts --- .../DorisColumnDefaultValueConverter.java | 12 ++- .../doris/operation/DorisTableOperations.java | 100 +++++++++++------- .../TestDorisColumnDefaultValueConverter.java | 5 +- ...TestDorisTableOperationsSqlGeneration.java | 66 +++++------- 4 files changed, 102 insertions(+), 81 deletions(-) diff --git a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java index 09eb63ec144..0e8771e54b7 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/converter/DorisColumnDefaultValueConverter.java @@ -32,6 +32,7 @@ import java.time.LocalDate; import java.time.LocalDateTime; +import javax.annotation.Nullable; import org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter; import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; import org.apache.gravitino.rel.expressions.Expression; @@ -88,8 +89,6 @@ public String fromGravitinoForAddColumn( return super.fromGravitino(defaultValue); } -<<<<<<< HEAD -======= /** * Converts a CREATE TABLE default value to Doris SQL. * @@ -168,7 +167,6 @@ public String fromGravitinoForModifyColumn( return super.fromGravitino(defaultValue); } ->>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) @Override public Expression toGravitino( JdbcTypeConverter.JdbcTypeBean columnType, @@ -243,6 +241,14 @@ private static String quoteDorisLiteral( return "\"" + escaped + "\""; } + private static String quoteDorisModifyLiteral( + String value, boolean doubleEscapeBackslashes, boolean tripleEscapeQuotes) { + String escapedBackslash = doubleEscapeBackslashes ? "\\".repeat(4) : "\\".repeat(2); + String escapedQuote = "\\".repeat(tripleEscapeQuotes ? 3 : 1) + "\""; + String escaped = value.replace("\\", escapedBackslash).replace("\"", escapedQuote); + return "\"" + escaped + "\""; + } + private static String unescapeDorisLiteral(String value) { StringBuilder result = new StringBuilder(value.length()); for (int i = 0; i < value.length(); i++) { diff --git a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java index 323bc0a0477..826c3ea5e88 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java @@ -76,6 +76,7 @@ import org.apache.gravitino.rel.indexes.Indexes; import org.apache.gravitino.rel.partitions.ListPartition; import org.apache.gravitino.rel.partitions.RangePartition; +import org.apache.gravitino.rel.types.Types; import org.apache.gravitino.utils.ExceptionMessages; /** Table operations for Apache Doris. */ @@ -849,6 +850,7 @@ protected String generateAlterTableSql( .anyMatch(DorisTableOperations::requiresVersionAwareAddColumnEscaping) ? Optional.of(getDorisVersion("ADD COLUMN default literal compatibility check")) : Optional.empty(); + Optional alterColumnDorisVersion = addColumnDorisVersion; for (int i = 0; i < changes.length; i++) { TableChange change = changes[i]; if (change instanceof TableChange.UpdateComment) { @@ -868,15 +870,11 @@ protected String generateAlterTableSql( } else if (change instanceof TableChange.UpdateColumnType) { lazyLoadTable = getOrCreateTable(databaseName, tableName, lazyLoadTable); TableChange.UpdateColumnType updateColumnType = (TableChange.UpdateColumnType) change; -<<<<<<< HEAD - alterSql.add(updateColumnTypeFieldDefinition(updateColumnType, lazyLoadTable)); -======= alterColumnDorisVersion = resolveModifyColumnVersion(updateColumnType, lazyLoadTable, alterColumnDorisVersion); alterSql.add( updateColumnTypeFieldDefinition( updateColumnType, lazyLoadTable, alterColumnDorisVersion)); ->>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) } else if (change instanceof TableChange.UpdateColumnComment) { TableChange.UpdateColumnComment updateColumnComment = (TableChange.UpdateColumnComment) change; @@ -945,34 +943,6 @@ protected String generateAlterTableSql( return result; } -<<<<<<< HEAD -======= - private static void validateIndexChangeConflicts(TableChange... changes) { - Set deleteIndexNames = new HashSet<>(); - Set addIndexNames = new HashSet<>(); - - for (TableChange change : changes) { - if (change instanceof TableChange.DeleteIndex) { - String indexName = ((TableChange.DeleteIndex) change).getName(); - Preconditions.checkArgument( - deleteIndexNames.add(indexName), - "Index '%s' cannot be deleted more than once in the same request", - indexName); - Preconditions.checkArgument( - !addIndexNames.contains(indexName), - "Index '%s' cannot be added and deleted in the same request", - indexName); - } else if (change instanceof TableChange.AddIndex) { - String indexName = ((TableChange.AddIndex) change).getName(); - Preconditions.checkArgument( - !deleteIndexNames.contains(indexName), - "Index '%s' cannot be added and deleted in the same request", - indexName); - addIndexNames.add(indexName); - } - } - } - private Optional resolveModifyColumnVersion( TableChange.ColumnChange change, JdbcTable table, Optional dorisVersion) { if (dorisVersion.isPresent() || change.fieldName().length != 1) { @@ -984,7 +954,6 @@ private Optional resolveModifyColumnVersion( : dorisVersion; } ->>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) private String updateColumnNullabilityDefinition( TableChange.UpdateColumnNullability change, JdbcTable table, Optional dorisVersion) { validateUpdateColumnNullable(change, table); @@ -1107,6 +1076,25 @@ private static boolean requiresVersionAwareAddColumnEscaping(Expression defaultV return stringValue.contains("\\") || stringValue.contains("\"\""); } + private static boolean requiresVersionAwareModifyEscaping(Column column) { + if (!(column.defaultValue() instanceof Literal)) { + return false; + } + + Literal literal = (Literal) column.defaultValue(); + if (!(literal.dataType() instanceof Types.StringType + || literal.dataType() instanceof Types.VarCharType + || literal.dataType() instanceof Types.FixedCharType)) { + return false; + } + Object value = literal.value(); + if (value == null) { + return false; + } + String stringValue = String.valueOf(value); + return stringValue.contains("\\") || stringValue.contains("\""); + } + private String serializeAddColumnDefaultValue( Expression defaultValue, Optional dorisVersion) { Preconditions.checkState( @@ -1179,7 +1167,9 @@ private String deleteColumnFieldDefinition( } private String updateColumnTypeFieldDefinition( - TableChange.UpdateColumnType updateColumnType, JdbcTable jdbcTable) { + TableChange.UpdateColumnType updateColumnType, + JdbcTable jdbcTable, + Optional dorisVersion) { if (updateColumnType.fieldName().length > 1) { throw new UnsupportedOperationException("Doris does not support nested column names."); } @@ -1191,11 +1181,49 @@ private String updateColumnTypeFieldDefinition( .withName(col) .withType(updateColumnType.getNewDataType()) .withComment(column.comment()) - .withDefaultValue(DEFAULT_VALUE_NOT_SET) + .withDefaultValue(column.defaultValue()) .withNullable(column.nullable()) .withAutoIncrement(column.autoIncrement()) .build(); - return appendColumnDefinition(newColumn, sqlBuilder).toString(); + return appendColumnDefinitionForModify(newColumn, sqlBuilder, dorisVersion).toString(); + } + + private StringBuilder appendColumnDefinitionForModify( + JdbcColumn column, StringBuilder sqlBuilder, Optional dorisVersion) { + sqlBuilder.append(SPACE).append(typeConverter.fromGravitino(column.dataType())).append(SPACE); + + if (column.nullable()) { + sqlBuilder.append("NULL "); + } else { + sqlBuilder.append("NOT NULL "); + } + + if (!DEFAULT_VALUE_NOT_SET.equals(column.defaultValue())) { + Preconditions.checkState( + columnDefaultValueConverter instanceof DorisColumnDefaultValueConverter, + "DorisColumnDefaultValueConverter is required for Doris MODIFY COLUMN"); + DorisColumnDefaultValueConverter converter = + (DorisColumnDefaultValueConverter) columnDefaultValueConverter; + boolean isDoris3x = + dorisVersion + .map( + version -> + isVersionAtLeast(version, 3, 0, 0) && !isVersionAtLeast(version, 4, 0, 0)) + .orElse(false); + String defaultValue = + converter.fromGravitinoForModifyColumn(column.defaultValue(), isDoris3x, isDoris3x); + Preconditions.checkState(defaultValue != null, "Doris default value must not be null"); + sqlBuilder.append("DEFAULT ").append(defaultValue).append(SPACE); + } + + if (column.autoIncrement()) { + sqlBuilder.append(DORIS_AUTO_INCREMENT).append(" "); + } + + if (StringUtils.isNotEmpty(column.comment())) { + sqlBuilder.append("COMMENT '").append(escapeSqlLiteral(column.comment(), '\'')).append("' "); + } + return sqlBuilder; } private StringBuilder appendColumnDefinition( diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java index cf87e64fcd8..19e47aff80a 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/converter/TestDorisColumnDefaultValueConverter.java @@ -18,9 +18,11 @@ */ package org.apache.gravitino.catalog.doris.converter; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_OF_CURRENT_TIMESTAMP; import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; +import org.apache.gravitino.rel.expressions.UnparsedExpression; import org.apache.gravitino.rel.expressions.literals.Literal; import org.apache.gravitino.rel.expressions.literals.Literals; import org.apache.gravitino.rel.types.Types; @@ -138,8 +140,6 @@ public void testUnescapeStringLikeDefaultValues() { Literals.of("owner's a\"\"b \"value\"\\path", Types.VarCharType.of(255)), CONVERTER.toGravitino(varcharType, "owner's a\"\"b \"value\"\\\\path", false, false)); } -<<<<<<< HEAD -======= /** Verifies loaded literal and native expression defaults use MODIFY COLUMN SQL forms. */ @Test @@ -190,5 +190,4 @@ public void testCreateTableStringDefaultLiteralForms() { Assertions.assertEquals( "12", converter.fromGravitinoForCreateTable(Literals.integerLiteral(12), true)); } ->>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) } diff --git a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java index 298fc00ff0a..ce4cc80cb1d 100644 --- a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java +++ b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java @@ -30,6 +30,7 @@ import java.sql.Statement; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import javax.sql.DataSource; import org.apache.gravitino.catalog.doris.converter.DorisColumnDefaultValueConverter; @@ -41,6 +42,7 @@ import org.apache.gravitino.rel.TableChange; import org.apache.gravitino.rel.expressions.Expression; import org.apache.gravitino.rel.expressions.NamedReference; +import org.apache.gravitino.rel.expressions.UnparsedExpression; import org.apache.gravitino.rel.expressions.distributions.Distribution; import org.apache.gravitino.rel.expressions.distributions.Distributions; import org.apache.gravitino.rel.expressions.literals.Literal; @@ -48,6 +50,7 @@ import org.apache.gravitino.rel.expressions.transforms.Transforms; import org.apache.gravitino.rel.indexes.Index; import org.apache.gravitino.rel.indexes.Indexes; +import org.apache.gravitino.rel.types.Type; import org.apache.gravitino.rel.types.Types; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -58,6 +61,8 @@ public class TestDorisTableOperationsSqlGeneration { private static class TestableDorisTableOperations extends DorisTableOperations { + private JdbcTable tableForAlter = JdbcTable.builder().withName("test_table").build(); + public TestableDorisTableOperations() { this("doris-3.0.6.2-rc01-910c4249c5"); } @@ -102,10 +107,31 @@ public String alterTableSql(String tableName, TableChange... changes) { return generateAlterTableSql("database", tableName, changes); } + /** + * Generates ALTER SQL while retaining the loaded column attributes. + * + * @param column the current column + * @param newType the requested type + * @return the generated SQL + */ + public String updateColumnTypeSql(JdbcColumn column, Type newType) { + setTableForAlter( + JdbcTable.builder() + .withName("test_table") + .withColumns(new JdbcColumn[] {column}) + .build()); + return alterTableSql( + "test_table", TableChange.updateColumnType(new String[] {column.name()}, newType)); + } + + void setTableForAlter(JdbcTable table) { + this.tableForAlter = table; + } + @Override protected JdbcTable getOrCreateTable( String databaseName, String tableName, JdbcTable lazyLoadCreateTable) { - return JdbcTable.builder().withName(tableName).build(); + return tableForAlter; } public String createTableSqlWithIndexes( @@ -119,13 +145,6 @@ public String createTableSqlWithIndexes( distribution, indexes); } -<<<<<<< HEAD -======= - - List indexes(Connection connection, String databaseName, String tableName) - throws SQLException { - return getIndexes(connection, databaseName, tableName); - } } /** Verifies string defaults and unchanged column attributes are preserved. */ @@ -339,23 +358,6 @@ public void testUpdateColumnTypePreservesNullAndUnsetDefaultDistinction() { ops.updateColumnTypeSql(unsetDefault, Types.LongType.get()).contains("bigint NULL ;")); } - /** - * Verifies rejected nested-column changes follow the alter-table exception contract. - * - * @param change the nested-column change to reject - */ - @ParameterizedTest - @MethodSource("nestedColumnChanges") - public void testRejectsNestedColumnName(TableChange change) { - TestableDorisTableOperations ops = new TestableDorisTableOperations(); - - IllegalArgumentException exception = - Assertions.assertThrows( - IllegalArgumentException.class, () -> ops.alterTableSql("test_table", change)); - - Assertions.assertEquals("Doris does not support nested column names.", exception.getMessage()); - } - /** Verifies version lookup is cached across type changes in one ALTER request. */ @Test public void testUpdateColumnTypeQueriesVersionOnceForAlterRequest() throws Exception { @@ -415,7 +417,6 @@ public void testAddAndUpdateColumnTypeShareVersionLookup() throws Exception { TableChange.updateColumnType(new String[] {"col1"}, Types.VarCharType.of(64))); Mockito.verify(versionDataSource, Mockito.times(1)).getConnection(); ->>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) } /** @@ -1338,18 +1339,6 @@ private static DataSource mockVersionDataSource(String dorisVersion) throws Exce Mockito.when(resultSet.getString(1)).thenReturn(dorisVersion); return dataSource; } -<<<<<<< HEAD -======= - - private static Stream nestedColumnChanges() { - String[] fieldName = {"parent", "child"}; - return Stream.of( - TableChange.addColumn(fieldName, Types.LongType.get()), - TableChange.updateColumnComment(fieldName, "new comment"), - TableChange.updateColumnPosition(fieldName, TableChange.ColumnPosition.first()), - TableChange.deleteColumn(fieldName, false), - TableChange.updateColumnType(fieldName, Types.LongType.get())); - } @Test public void testSetPropertyEscapesKeyAndValue() { @@ -1365,5 +1354,4 @@ public void testSetPropertyEscapesKeyAndValue() { + "\"plain\" = \"value\");", sql); } ->>>>>>> 4e900e8f1 ([#13973][#13974] fix(catalog-jdbc): Escape column default literals and table properties (#13975)) }