This is an automated email from the ASF dual-hosted git repository.
mchades pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new ef81d1e375 [Cherry-pick to branch-1.3] [MINOR] fix(jdbc): Quote
comments in generated DDL (#12007) (#12122)
ef81d1e375 is described below
commit ef81d1e3755caeabd4d82c61506faf0704e4f82c
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Jul 21 19:45:00 2026 +0800
[Cherry-pick to branch-1.3] [MINOR] fix(jdbc): Quote comments in generated
DDL (#12007) (#12122)
**Cherry-pick Information:**
- Original commit: afbd958c45c3007d7324a69e402a9d126ad3d361
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Qi Yu <[email protected]>
---
.../catalog/jdbc/utils/JdbcConnectorUtils.java | 61 +++++++++++++++++
.../catalog/jdbc/utils/TestJdbcConnectorUtils.java | 69 +++++++++++++++++++
.../doris/operation/DorisTableOperations.java | 15 +++--
.../gravitino/catalog/doris/utils/DorisUtils.java | 17 ++++-
.../operation/TestDorisDatabaseOperations.java | 2 +-
.../TestDorisDatabaseOperationsSqlGeneration.java | 39 +++++++++++
.../TestDorisTableOperationsSqlGeneration.java | 63 +++++++++++++++++-
.../catalog/doris/utils/TestDorisUtils.java | 21 ++++++
.../mysql/operation/MysqlTableOperations.java | 12 ++--
.../TestMysqlTableOperationsSqlGeneration.java | 60 ++++++++++++++++-
.../operation/PostgreSqlSchemaOperations.java | 5 +-
.../operation/PostgreSqlTableOperations.java | 20 ++++--
...estPostgreSqlSchemaOperationsSqlGeneration.java | 40 +++++++++++
...TestPostgreSqlTableOperationsSqlGeneration.java | 59 ++++++++++++++++-
.../operations/StarRocksTableOperations.java | 22 +++++--
.../catalog/starrocks/utils/StarRocksUtils.java | 7 +-
.../operation/TestStarRocksTableOperations.java | 50 ++++++++++++++
.../TestStarRocksTableOperationsSqlGeneration.java | 77 +++++++++++++++++++++-
.../starrocks/utils/TestStarRocksUtils.java | 25 +++++++
19 files changed, 633 insertions(+), 31 deletions(-)
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/JdbcConnectorUtils.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/JdbcConnectorUtils.java
index 62ab29610f..b4d1a8dd92 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/JdbcConnectorUtils.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/JdbcConnectorUtils.java
@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
import java.sql.Connection;
import java.sql.SQLException;
import java.sql.Statement;
+import javax.annotation.Nullable;
public final class JdbcConnectorUtils {
public static final ImmutableList<String> TABLE_TYPES =
ImmutableList.of("TABLE");
@@ -43,6 +44,66 @@ public final class JdbcConnectorUtils {
}
}
+ /**
+ * Escapes a value for inclusion in a quoted SQL string literal.
+ *
+ * <p>This method escapes both backslashes and the selected quote character.
Callers are
+ * responsible for adding the surrounding quote characters and any
dialect-specific literal
+ * prefix.
+ *
+ * @param value the literal value to escape; {@code null} is represented as
the text {@code null}
+ * @param quote the quote character used by the SQL dialect
+ * @return the escaped literal value
+ * @throws IllegalArgumentException if {@code quote} is neither a single nor
double quote
+ */
+ public static String escapeSqlLiteral(@Nullable String value, char quote) {
+ if (quote != '\'' && quote != '"') {
+ throw new IllegalArgumentException("SQL literal quote must be a single
or double quote");
+ }
+
+ String literalValue = String.valueOf(value);
+ String quoteString = String.valueOf(quote);
+ return literalValue.replace("\\", "\\\\").replace(quoteString, quoteString
+ quoteString);
+ }
+
+ /**
+ * Reverses {@link #escapeSqlLiteral(String, char)} for text captured from a
quoted SQL string
+ * literal, e.g. when parsing {@code SHOW CREATE} output.
+ *
+ * <p>Both doubled-quote ({@code ""} or {@code ''}) and backslash ({@code
\"}, {@code \\}) escape
+ * styles are unescaped, since dialects differ in which form they emit. Any
other character is
+ * kept as is, so text that was never escaped passes through unchanged.
+ *
+ * @param value the captured literal text without the surrounding quote
characters
+ * @param quote the quote character used by the SQL dialect
+ * @return the unescaped literal value
+ * @throws IllegalArgumentException if {@code quote} is neither a single nor
double quote
+ */
+ public static String unescapeSqlLiteral(String value, char quote) {
+ if (quote != '\'' && quote != '"') {
+ throw new IllegalArgumentException("SQL literal quote must be a single
or double quote");
+ }
+
+ StringBuilder result = new StringBuilder(value.length());
+ for (int i = 0; i < value.length(); i++) {
+ char current = value.charAt(i);
+ if (current == '\\' && i + 1 < value.length()) {
+ char next = value.charAt(i + 1);
+ if (next == '\\' || next == quote) {
+ result.append(next);
+ i++;
+ continue;
+ }
+ } else if (current == quote && i + 1 < value.length() && value.charAt(i
+ 1) == quote) {
+ result.append(quote);
+ i++;
+ continue;
+ }
+ result.append(current);
+ }
+ return result.toString();
+ }
+
public static String[] getTableTypes() {
return TABLE_TYPES.toArray(new String[0]);
}
diff --git
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestJdbcConnectorUtils.java
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestJdbcConnectorUtils.java
new file mode 100644
index 0000000000..26c91d0564
--- /dev/null
+++
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestJdbcConnectorUtils.java
@@ -0,0 +1,69 @@
+/*
+ * 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.jdbc.utils;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class TestJdbcConnectorUtils {
+
+ @Test
+ public void testEscapeSqlLiteral() {
+ String value = "owner\\'s \"comment\"; DROP TABLE marker; --";
+
+ Assertions.assertEquals(
+ "owner\\\\''s \"comment\"; DROP TABLE marker; --",
+ JdbcConnectorUtils.escapeSqlLiteral(value, '\''));
+ Assertions.assertEquals(
+ "owner\\\\'s \"\"comment\"\"; DROP TABLE marker; --",
+ JdbcConnectorUtils.escapeSqlLiteral(value, '"'));
+ Assertions.assertThrows(
+ IllegalArgumentException.class, () ->
JdbcConnectorUtils.escapeSqlLiteral(value, '`'));
+ Assertions.assertEquals("null", JdbcConnectorUtils.escapeSqlLiteral(null,
'\''));
+ }
+
+ @Test
+ public void testUnescapeSqlLiteral() {
+ String value = "owner\\'s \"comment\"; DROP TABLE marker; --";
+
+ // Round-trip: unescape(escape(x)) == x for both quote styles.
+ Assertions.assertEquals(
+ value,
+ JdbcConnectorUtils.unescapeSqlLiteral(
+ JdbcConnectorUtils.escapeSqlLiteral(value, '\''), '\''));
+ Assertions.assertEquals(
+ value,
+ JdbcConnectorUtils.unescapeSqlLiteral(
+ JdbcConnectorUtils.escapeSqlLiteral(value, '"'), '"'));
+
+ // Backslash-style escaping is unescaped too.
+ Assertions.assertEquals(
+ "owner's \"comment\"",
+ JdbcConnectorUtils.unescapeSqlLiteral("owner's \\\"comment\\\"", '"'));
+
+ // Text that was never escaped passes through unchanged, including a lone
backslash before an
+ // ordinary character and a lone trailing backslash.
+ Assertions.assertEquals(
+ "D:\\data; --", JdbcConnectorUtils.unescapeSqlLiteral("D:\\data; --",
'"'));
+ Assertions.assertEquals("tail\\",
JdbcConnectorUtils.unescapeSqlLiteral("tail\\", '"'));
+
+ Assertions.assertThrows(
+ IllegalArgumentException.class, () ->
JdbcConnectorUtils.unescapeSqlLiteral(value, '`'));
+ }
+}
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 e82e44240b..926c193267 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
@@ -22,6 +22,7 @@ import static
org.apache.gravitino.catalog.doris.DorisCatalog.DORIS_TABLE_PROPER
import static
org.apache.gravitino.catalog.doris.DorisTablePropertiesMetadata.DEFAULT_REPLICATION_FACTOR_IN_SERVER_SIDE;
import static
org.apache.gravitino.catalog.doris.DorisTablePropertiesMetadata.REPLICATION_FACTOR;
import static
org.apache.gravitino.catalog.doris.utils.DorisUtils.generatePartitionSqlFragment;
+import static
org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.escapeSqlLiteral;
import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET;
import com.google.common.annotations.VisibleForTesting;
@@ -136,7 +137,7 @@ public class DorisTableOperations extends
JdbcTableOperations {
// Add table comment if specified
if (StringUtils.isNotEmpty(comment)) {
- sqlBuilder.append(" COMMENT \"").append(comment).append("\"");
+ sqlBuilder.append(" COMMENT \"").append(escapeSqlLiteral(comment,
'"')).append("\"");
}
// Add Partition Info
@@ -796,7 +797,7 @@ public class DorisTableOperations extends
JdbcTableOperations {
newComment = StringIdentifier.addToComment(identifier, newComment);
}
}
- alterSql.add("MODIFY COMMENT \"" + newComment + "\"");
+ alterSql.add("MODIFY COMMENT \"" + escapeSqlLiteral(newComment, '"') +
"\"");
}
if (CollectionUtils.isEmpty(alterSql)) {
@@ -848,7 +849,8 @@ public class DorisTableOperations extends
JdbcTableOperations {
}
String col = updateColumnComment.fieldName()[0];
- return String.format("MODIFY COLUMN `%s` COMMENT '%s'", col, newComment);
+ return String.format(
+ "MODIFY COLUMN `%s` COMMENT '%s'", col, escapeSqlLiteral(newComment,
'\''));
}
private String addColumnFieldDefinition(TableChange.AddColumn addColumn) {
@@ -873,7 +875,10 @@ public class DorisTableOperations extends
JdbcTableOperations {
}
// Append comment if available
if (StringUtils.isNotEmpty(addColumn.getComment())) {
- columnDefinition.append("COMMENT
'").append(addColumn.getComment()).append("' ");
+ columnDefinition
+ .append("COMMENT '")
+ .append(escapeSqlLiteral(addColumn.getComment(), '\''))
+ .append("' ");
}
// Append position if available
@@ -985,7 +990,7 @@ public class DorisTableOperations extends
JdbcTableOperations {
// Add column comment if specified
if (StringUtils.isNotEmpty(column.comment())) {
- sqlBuilder.append("COMMENT '").append(column.comment()).append("' ");
+ sqlBuilder.append("COMMENT '").append(escapeSqlLiteral(column.comment(),
'\'')).append("' ");
}
return sqlBuilder;
}
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 03d4dd136e..80a376411e 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
@@ -18,6 +18,9 @@
*/
package org.apache.gravitino.catalog.doris.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 java.util.Arrays;
import java.util.HashMap;
@@ -65,7 +68,13 @@ public final class DorisUtils {
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();
@@ -87,8 +96,10 @@ public final class DorisUtils {
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).trim(), '"');
+ String value = unescapeSqlLiteral(matcherProperties.group(2).trim(),
'"');
properties.put(key, value);
}
}
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 2ea78fac7b..6713452816 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
@@ -33,7 +33,7 @@ public class TestDorisDatabaseOperations extends TestDoris {
@Test
public void testBaseOperationDatabase() {
String databaseName = RandomNameUtils.genRandomName("it_db");
- String comment = "comment";
+ String comment = "owner's \"comment\" C:\\tmp; DROP DATABASE marker; --";
Map<String, String> properties = new HashMap<>();
properties.put("property1", "value1");
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseOperationsSqlGeneration.java
new file mode 100644
index 0000000000..3f995a21c8
--- /dev/null
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisDatabaseOperationsSqlGeneration.java
@@ -0,0 +1,39 @@
+/*
+ * 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 java.util.Collections;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class TestDorisDatabaseOperationsSqlGeneration {
+
+ @Test
+ public void testEscapeCommentInGeneratedSql() {
+ DorisDatabaseOperations operations = new DorisDatabaseOperations();
+ String comment = "owner's \"comment\" C:\\tmp; DROP DATABASE marker; --";
+
+ String sql =
+ operations.generateCreateDatabaseSql("test_database", comment,
Collections.emptyMap());
+
+ Assertions.assertTrue(
+ sql.contains("\"comment\"=\"owner's \"\"comment\"\" C:\\\\tmp; DROP
DATABASE marker; --\""),
+ sql);
+ }
+}
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 705b23a735..68b442bbe9 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
@@ -73,16 +73,31 @@ public class TestDorisTableOperationsSqlGeneration {
public String createTableSql(
String tableName, JdbcColumn[] columns, Distribution distribution) {
+ return createTableSql(tableName, columns, distribution, "comment");
+ }
+
+ public String createTableSql(
+ String tableName, JdbcColumn[] columns, Distribution distribution,
String comment) {
return generateCreateTableSql(
tableName,
columns,
- "comment",
+ comment,
Collections.emptyMap(),
Transforms.EMPTY_TRANSFORM,
distribution,
Indexes.EMPTY_INDEXES);
}
+ public String alterTableSql(String tableName, TableChange... changes) {
+ return generateAlterTableSql("database", tableName, changes);
+ }
+
+ @Override
+ protected JdbcTable getOrCreateTable(
+ String databaseName, String tableName, JdbcTable lazyLoadCreateTable) {
+ return JdbcTable.builder().withName(tableName).build();
+ }
+
public String createTableSqlWithIndexes(
String tableName, JdbcColumn[] columns, Distribution distribution,
Index[] indexes) {
return generateCreateTableSql(
@@ -148,6 +163,52 @@ public class TestDorisTableOperationsSqlGeneration {
"Should contain DEFAULT value but was: " + sql);
}
+ @Test
+ public void testEscapeCommentsInGeneratedSql() {
+ TestableDorisTableOperations ops = new TestableDorisTableOperations();
+ TestableDorisTableOperations mockOps = Mockito.spy(ops);
+ Mockito.doAnswer(a -> a.getArgument(0))
+ .when(mockOps)
+ .appendNecessaryProperties(Mockito.anyMap());
+ String tableComment = "owner\"; DROP TABLE marker; --";
+ String columnComment = "owner's comment; DROP TABLE marker; --";
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col1")
+ .withType(Types.IntegerType.get())
+ .withComment(columnComment)
+ .build();
+ Distribution distribution = Distributions.hash(1,
NamedReference.field("col1"));
+
+ String createSql =
+ mockOps.createTableSql("test_table", new JdbcColumn[] {column},
distribution, tableComment);
+ Assertions.assertTrue(
+ createSql.contains("COMMENT \"owner\"\"; DROP TABLE marker; --\""),
createSql);
+ Assertions.assertTrue(
+ createSql.contains("COMMENT 'owner''s comment; DROP TABLE marker;
--'"), createSql);
+
+ String alterSql =
+ mockOps.alterTableSql(
+ "test_table", TableChange.updateColumnComment(new String[]
{"col1"}, columnComment));
+ Assertions.assertTrue(
+ alterSql.contains("MODIFY COLUMN `col1` COMMENT 'owner''s comment;
DROP TABLE marker; --'"),
+ alterSql);
+ }
+
+ @Test
+ public void testEscapeAddColumnCommentInGeneratedSql() {
+ TestableDorisTableOperations ops = new TestableDorisTableOperations();
+ String comment = "owner\\'s \"comment\"; --";
+
+ String alterSql =
+ ops.alterTableSql(
+ "test_table",
+ TableChange.addColumn(new String[] {"col2"},
Types.IntegerType.get(), comment));
+
+ Assertions.assertTrue(alterSql.contains("ADD COLUMN `col2`"), alterSql);
+ Assertions.assertTrue(alterSql.contains("COMMENT 'owner\\\\''s
\"comment\"; --'"), alterSql);
+ }
+
@Test
public void testCreateTableWithPrimaryKeyIndex() {
TestableDorisTableOperations ops = new TestableDorisTableOperations();
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 c26f025beb..4a2b37981e 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
@@ -54,6 +54,11 @@ public class TestDorisUtils {
result = DorisUtils.generatePropertiesSql(properties);
assertEquals(" PROPERTIES (\n\"key\"=\"value\"\n)", result);
+ properties = Collections.singletonMap("key\"name", "owner's \"comment\"
C:\\tmp; --");
+ result = DorisUtils.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");
@@ -92,6 +97,22 @@ public class TestDorisUtils {
result = DorisUtils.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 = DorisUtils.extractPropertiesFromSql(createTableSql);
+ assertEquals("owner's \"comment\" D:\\data; --", result.get("key\"name"));
+
+ // Backslash-style escaping (\" and \\) is unescaped too, and a lone
backslash before an
+ // ordinary character passes through unchanged.
+ createTableSql =
+ "CREATE DATABASE `test`\nPROPERTIES (\n"
+ + "\"plain\" = \"owner\\\\s \\\"comment\\\" D:\\data; --\"\n)";
+ result = DorisUtils.extractPropertiesFromSql(createTableSql);
+ assertEquals("owner\\s \"comment\" D:\\data; --", result.get("plain"));
}
@Test
diff --git
a/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
b/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
index 0a597ef214..f30ef31f4d 100644
---
a/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
+++
b/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog.mysql.operation;
+import static
org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.escapeSqlLiteral;
import static
org.apache.gravitino.catalog.mysql.MysqlTablePropertiesMetadata.MYSQL_AUTO_INCREMENT_OFFSET_KEY;
import static
org.apache.gravitino.catalog.mysql.MysqlTablePropertiesMetadata.MYSQL_ENGINE_KEY;
import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET;
@@ -113,7 +114,7 @@ public class MysqlTableOperations extends
JdbcTableOperations {
// Add table comment if specified
if (StringUtils.isNotEmpty(comment)) {
- sqlBuilder.append(" COMMENT='").append(comment).append("'");
+ sqlBuilder.append(" COMMENT='").append(escapeSqlLiteral(comment,
'\'')).append("'");
}
// Add table properties if any
@@ -295,7 +296,7 @@ public class MysqlTableOperations extends
JdbcTableOperations {
newComment = StringIdentifier.addToComment(identifier, newComment);
}
}
- alterSql.add("COMMENT '" + newComment + "'");
+ alterSql.add("COMMENT '" + escapeSqlLiteral(newComment, '\'') + "'");
}
if (!setProperties.isEmpty()) {
@@ -460,7 +461,10 @@ public class MysqlTableOperations extends
JdbcTableOperations {
}
// Append comment if available
if (StringUtils.isNotEmpty(addColumn.getComment())) {
- columnDefinition.append("COMMENT
'").append(addColumn.getComment()).append("' ");
+ columnDefinition
+ .append("COMMENT '")
+ .append(escapeSqlLiteral(addColumn.getComment(), '\''))
+ .append("' ");
}
// Append default value if available
@@ -626,7 +630,7 @@ public class MysqlTableOperations extends
JdbcTableOperations {
// Add column comment if specified
if (StringUtils.isNotEmpty(column.comment())) {
- sqlBuilder.append("COMMENT '").append(column.comment()).append("' ");
+ sqlBuilder.append("COMMENT '").append(escapeSqlLiteral(column.comment(),
'\'')).append("' ");
}
return sqlBuilder;
}
diff --git
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperationsSqlGeneration.java
index 2727feee23..ad8966c2c8 100644
---
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperationsSqlGeneration.java
@@ -20,9 +20,11 @@ package org.apache.gravitino.catalog.mysql.operation;
import java.util.Collections;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
+import org.apache.gravitino.catalog.jdbc.JdbcTable;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
import org.apache.gravitino.catalog.mysql.converter.MysqlTypeConverter;
+import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.literals.Literals;
import org.apache.gravitino.rel.expressions.transforms.Transforms;
@@ -41,15 +43,29 @@ public class TestMysqlTableOperationsSqlGeneration {
}
public String createTableSql(String tableName, JdbcColumn[] columns) {
+ return createTableSql(tableName, columns, "comment");
+ }
+
+ public String createTableSql(String tableName, JdbcColumn[] columns,
String comment) {
return generateCreateTableSql(
tableName,
columns,
- "comment",
+ comment,
Collections.emptyMap(),
Transforms.EMPTY_TRANSFORM,
Distributions.NONE,
Indexes.EMPTY_INDEXES);
}
+
+ public String alterTableSql(String tableName, TableChange... changes) {
+ return generateAlterTableSql("database", tableName, changes);
+ }
+
+ @Override
+ protected JdbcTable getOrCreateTable(
+ String databaseName, String tableName, JdbcTable lazyLoadCreateTable) {
+ return JdbcTable.builder().withName(tableName).build();
+ }
}
@Test
@@ -89,4 +105,46 @@ public class TestMysqlTableOperationsSqlGeneration {
sql.contains("DEFAULT " +
converter.fromGravitino(col1.defaultValue())),
"Should contain DEFAULT value but was: " + sql);
}
+
+ @Test
+ public void testEscapeCommentsInGeneratedSql() {
+ TestableMysqlTableOperations ops = new TestableMysqlTableOperations();
+ String injectedComment = "owner's comment; DROP TABLE marker; --";
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col1")
+ .withType(Types.IntegerType.get())
+ .withComment(injectedComment)
+ .build();
+
+ String createSql = ops.createTableSql("test_table", new JdbcColumn[]
{column}, injectedComment);
+ Assertions.assertTrue(
+ createSql.contains("COMMENT 'owner''s comment; DROP TABLE marker;
--'"), createSql);
+ Assertions.assertTrue(
+ createSql.contains("COMMENT='owner''s comment; DROP TABLE marker;
--'"), createSql);
+
+ String commentWithIdentifier =
+ injectedComment + " (From Gravitino, DO NOT EDIT: gravitino.v1.uid1)";
+ String alterSql =
+ ops.alterTableSql("test_table",
TableChange.updateComment(commentWithIdentifier));
+ Assertions.assertTrue(
+ alterSql.contains(
+ "COMMENT 'owner''s comment; DROP TABLE marker; -- "
+ + "(From Gravitino, DO NOT EDIT: gravitino.v1.uid1)'"),
+ alterSql);
+ }
+
+ @Test
+ public void testEscapeAddColumnCommentInGeneratedSql() {
+ TestableMysqlTableOperations ops = new TestableMysqlTableOperations();
+ String comment = "owner\\'s \"comment\"; --";
+
+ String alterSql =
+ ops.alterTableSql(
+ "test_table",
+ TableChange.addColumn(new String[] {"col2"},
Types.IntegerType.get(), comment));
+
+ Assertions.assertTrue(alterSql.contains("ADD COLUMN `col2`"), alterSql);
+ Assertions.assertTrue(alterSql.contains("COMMENT 'owner\\\\''s
\"comment\"; --'"), alterSql);
+ }
}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java
index 1df76e914d..3996297b01 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlSchemaOperations.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog.postgresql.operation;
+import static
org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.escapeSqlLiteral;
import static
org.apache.gravitino.catalog.postgresql.operation.PostgreSqlTableOperations.PG_QUOTE;
import com.google.common.collect.ImmutableSet;
@@ -105,8 +106,8 @@ public class PostgreSqlSchemaOperations extends
JdbcDatabaseOperations {
.append(PG_QUOTE)
.append(schema)
.append(PG_QUOTE)
- .append(" IS '")
- .append(comment)
+ .append(" IS E'")
+ .append(escapeSqlLiteral(comment, '\''))
.append("'");
}
return sqlBuilder.toString();
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
index a47e136e05..83be0efcaa 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
@@ -18,6 +18,8 @@
*/
package org.apache.gravitino.catalog.postgresql.operation;
+import static
org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.escapeSqlLiteral;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
@@ -65,7 +67,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
public static final String NEW_LINE = "\n";
public static final String ALTER_TABLE = "ALTER TABLE ";
public static final String ALTER_COLUMN = "ALTER COLUMN ";
- public static final String IS = " IS '";
+ public static final String IS = " IS E'";
public static final String COLUMN_COMMENT = "COMMENT ON COLUMN ";
public static final String TABLE_COMMENT = "COMMENT ON TABLE ";
@@ -204,7 +206,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
.append(tableName)
.append(PG_QUOTE)
.append(IS)
- .append(comment)
+ .append(escapeSqlLiteral(comment, '\''))
.append("';");
}
Arrays.stream(columns)
@@ -222,7 +224,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
.append(jdbcColumn.name())
.append(PG_QUOTE)
.append(IS)
- .append(jdbcColumn.comment())
+ .append(escapeSqlLiteral(jdbcColumn.comment(), '\''))
.append("';"));
// Return the generated SQL statement
@@ -510,7 +512,13 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
}
}
}
- return TABLE_COMMENT + PG_QUOTE + jdbcTable.name() + PG_QUOTE + IS +
newComment + "';";
+ return TABLE_COMMENT
+ + PG_QUOTE
+ + jdbcTable.name()
+ + PG_QUOTE
+ + IS
+ + escapeSqlLiteral(newComment, '\'')
+ + "';";
}
private String deleteColumnFieldDefinition(
@@ -689,7 +697,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
+ col
+ PG_QUOTE
+ IS
- + addColumn.getComment()
+ + escapeSqlLiteral(addColumn.getComment(), '\'')
+ "';");
}
return result;
@@ -711,7 +719,7 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
+ col
+ PG_QUOTE
+ IS
- + newComment
+ + escapeSqlLiteral(newComment, '\'')
+ "';";
}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperationsSqlGeneration.java
new file mode 100644
index 0000000000..c961b49537
--- /dev/null
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlSchemaOperationsSqlGeneration.java
@@ -0,0 +1,40 @@
+/*
+ * 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.operation;
+
+import java.util.Collections;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class TestPostgreSqlSchemaOperationsSqlGeneration {
+
+ @Test
+ public void testEscapeCommentInGeneratedSql() {
+ PostgreSqlSchemaOperations operations = new PostgreSqlSchemaOperations();
+
+ String sql =
+ operations.generateCreateDatabaseSql(
+ "test_schema", "owner\\'s comment; DROP SCHEMA marker; --",
Collections.emptyMap());
+
+ Assertions.assertEquals(
+ "CREATE SCHEMA \"test_schema\";COMMENT ON SCHEMA \"test_schema\" "
+ + "IS E'owner\\\\''s comment; DROP SCHEMA marker; --'",
+ sql);
+ }
+}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
index 12f2e1834e..f428271c6a 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
@@ -20,9 +20,11 @@ package org.apache.gravitino.catalog.postgresql.operation;
import java.util.Collections;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
+import org.apache.gravitino.catalog.jdbc.JdbcTable;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
import
org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter;
+import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.literals.Literals;
import org.apache.gravitino.rel.expressions.transforms.Transforms;
@@ -41,15 +43,29 @@ public class TestPostgreSqlTableOperationsSqlGeneration {
}
public String createTableSql(String tableName, JdbcColumn[] columns) {
+ return createTableSql(tableName, columns, "comment");
+ }
+
+ public String createTableSql(String tableName, JdbcColumn[] columns,
String comment) {
return generateCreateTableSql(
tableName,
columns,
- "comment",
+ comment,
Collections.emptyMap(),
Transforms.EMPTY_TRANSFORM,
Distributions.NONE,
Indexes.EMPTY_INDEXES);
}
+
+ public String alterTableSql(String tableName, TableChange... changes) {
+ return generateAlterTableSql("database", tableName, changes);
+ }
+
+ @Override
+ protected JdbcTable getOrCreateTable(
+ String databaseName, String tableName, JdbcTable lazyLoadCreateTable) {
+ return JdbcTable.builder().withName(tableName).build();
+ }
}
@Test
@@ -89,4 +105,45 @@ public class TestPostgreSqlTableOperationsSqlGeneration {
sql.contains("DEFAULT " +
converter.fromGravitino(col1.defaultValue())),
"Should contain DEFAULT value but was: " + sql);
}
+
+ @Test
+ public void testEscapeCommentsInGeneratedSql() {
+ TestablePostgreSqlTableOperations ops = new
TestablePostgreSqlTableOperations();
+ String injectedComment = "owner\\'s comment; DROP TABLE marker; --";
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col1")
+ .withType(Types.IntegerType.get())
+ .withComment(injectedComment)
+ .build();
+
+ String createSql = ops.createTableSql("test_table", new JdbcColumn[]
{column}, injectedComment);
+ Assertions.assertTrue(
+ createSql.contains("IS E'owner\\\\''s comment; DROP TABLE marker;
--';"), createSql);
+
+ String alterSql =
+ ops.alterTableSql(
+ "test_table", TableChange.updateColumnComment(new String[]
{"col1"}, injectedComment));
+ Assertions.assertEquals(
+ "COMMENT ON COLUMN \"test_table\".\"col1\" "
+ + "IS E'owner\\\\''s comment; DROP TABLE marker; --';",
+ alterSql);
+ }
+
+ @Test
+ public void testEscapeAddColumnCommentInGeneratedSql() {
+ TestablePostgreSqlTableOperations ops = new
TestablePostgreSqlTableOperations();
+ String comment = "owner\\'s \"comment\"; --";
+
+ String alterSql =
+ ops.alterTableSql(
+ "test_table",
+ TableChange.addColumn(new String[] {"col2"},
Types.IntegerType.get(), comment));
+
+ Assertions.assertTrue(alterSql.contains("ADD COLUMN \"col2\""), alterSql);
+ Assertions.assertTrue(
+ alterSql.contains(
+ "COMMENT ON COLUMN \"test_table\".\"col2\" IS E'owner\\\\''s
\"comment\"; --';"),
+ alterSql);
+ }
}
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 f4dc067ba3..e123be6103 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
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog.starrocks.operations;
+import static
org.apache.gravitino.catalog.jdbc.utils.JdbcConnectorUtils.escapeSqlLiteral;
import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET;
import com.google.common.base.Preconditions;
@@ -118,7 +119,7 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
sqlBuilder.append(")\n");
if (StringUtils.isNotEmpty(comment)) {
comment = StringIdentifier.addToComment(StringIdentifier.DUMMY_ID,
comment);
- sqlBuilder.append(" COMMENT \"").append(comment).append("\"");
+ sqlBuilder.append(" COMMENT \"").append(escapeSqlLiteral(comment,
'"')).append("\"");
}
appendPartitionSql(partitioning, columns, sqlBuilder);
@@ -170,7 +171,17 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
} else if (change instanceof TableChange.UpdateComment) {
TableChange.UpdateComment updateComment = (TableChange.UpdateComment)
change;
String newComment = updateComment.getNewComment();
- alterSql.add("MODIFY COMMENT \"" + newComment + "\"");
+ if (StringIdentifier.fromComment(newComment) == null) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ StringIdentifier identifier =
StringIdentifier.fromComment(lazyLoadTable.comment());
+ if (identifier != null) {
+ newComment = StringIdentifier.addToComment(identifier, newComment);
+ }
+ }
+ if (StringUtils.isNotEmpty(newComment)) {
+ newComment =
StringIdentifier.addToComment(StringIdentifier.DUMMY_ID, newComment);
+ }
+ alterSql.add("COMMENT = \"" + escapeSqlLiteral(newComment, '"') +
"\"");
} else if (change instanceof TableChange.SetProperty) {
if (hasSetPropertyChange) {
throw new IllegalArgumentException(
@@ -333,7 +344,7 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
// Add column comment if specified
if (StringUtils.isNotEmpty(column.comment())) {
- sqlBuilder.append("COMMENT '").append(column.comment()).append("' ");
+ sqlBuilder.append("COMMENT '").append(escapeSqlLiteral(column.comment(),
'\'')).append("' ");
}
return sqlBuilder;
}
@@ -481,7 +492,10 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
// Append comment if available
if (StringUtils.isNotEmpty(addColumn.getComment())) {
- columnDefinition.append("COMMENT
'").append(addColumn.getComment()).append("' ");
+ columnDefinition
+ .append("COMMENT '")
+ .append(escapeSqlLiteral(addColumn.getComment(), '\''))
+ .append("' ");
}
// Append position if available
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 f97e2e316f..a918b26dc0 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
@@ -30,6 +30,7 @@ import java.util.Optional;
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;
@@ -67,7 +68,9 @@ public class StarRocksUtils {
"(?:^|\\s|\\))DISTRIBUTED\\s+BY\\s+(?:RANDOM\\b|\\w+\\s*\\()",
Pattern.CASE_INSENSITIVE);
private static final Pattern TABLE_COMMENT_PATTERN =
- Pattern.compile("COMMENT\\s*\"([^\\(]+?)\\s*\\(From Gravitino,.*\\)\"");
+ Pattern.compile(
+ "COMMENT\\s*\"((?:\\\\.|\"\"|[^\"\\\\])*)\\s+"
+ + "\\(From Gravitino, DO NOT EDIT:
gravitino\\.v\\d+\\.uid-?\\d+\\)\"");
private static final String PARTITION_TYPE_VALUE_PATTERN_STRING =
"types: \\[([^\\]]+)\\]; keys: \\[([^\\]]+)\\];";
@@ -196,7 +199,7 @@ public class StarRocksUtils {
public static String extractTableCommentFromSql(String createTableSql) {
Matcher matcher = TABLE_COMMENT_PATTERN.matcher(createTableSql.trim());
if (matcher.find()) {
- return matcher.group(1);
+ return JdbcConnectorUtils.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 b7726b96fd..5f531e7d55 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
@@ -31,6 +31,7 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
+import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
import org.apache.gravitino.catalog.jdbc.JdbcTable;
import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
@@ -195,6 +196,55 @@ public class TestStarRocksTableOperations extends
TestStarRocks {
TABLE_OPERATIONS.drop(databaseName, newName), "table should be
non-existent");
}
+ @Test
+ public void testTableCommentWithSqlLiteralCharacters() {
+ String tableName =
GravitinoITUtils.genRandomName("starrocks_comment_test_table");
+ String createComment = "owner's \"comment\" (created) C:\\tmp; --";
+ String updatedComment = "reviewer's \"comment\" (updated) D:\\data; --";
+ StringIdentifier identifier = StringIdentifier.fromId(42);
+ JdbcColumn column =
+
JdbcColumn.builder().withName("col_1").withType(INT).withComment("id").build();
+
+ try {
+ TABLE_OPERATIONS.create(
+ databaseName,
+ tableName,
+ new JdbcColumn[] {column},
+ StringIdentifier.addToComment(identifier, createComment),
+ createProperties(),
+ null,
+ Distributions.hash(DEFAULT_BUCKET_SIZE,
NamedReference.field("col_1")),
+ Indexes.EMPTY_INDEXES);
+
+ Assertions.assertEquals(
+ StringIdentifier.addToComment(identifier, createComment),
+ TABLE_OPERATIONS.load(databaseName, tableName).comment());
+
+ TABLE_OPERATIONS.alterTable(
+ databaseName, tableName, TableChange.updateComment(updatedComment));
+ Awaitility.await()
+ .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS)
+ .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS)
+ .untilAsserted(
+ () ->
+ Assertions.assertEquals(
+ StringIdentifier.addToComment(identifier,
updatedComment),
+ TABLE_OPERATIONS.load(databaseName,
tableName).comment()));
+
+ TABLE_OPERATIONS.alterTable(databaseName, tableName,
TableChange.updateComment(""));
+ Awaitility.await()
+ .atMost(MAX_WAIT_IN_SECONDS, TimeUnit.SECONDS)
+ .pollInterval(WAIT_INTERVAL_IN_SECONDS, TimeUnit.SECONDS)
+ .untilAsserted(
+ () ->
+ Assertions.assertEquals(
+ StringIdentifier.addToComment(identifier, ""),
+ TABLE_OPERATIONS.load(databaseName,
tableName).comment()));
+ } finally {
+ TABLE_OPERATIONS.drop(databaseName, tableName);
+ }
+ }
+
@Test
public void testAlterTable() {
String tableName =
GravitinoITUtils.genRandomName("starrocks_alter_test_table");
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 f8eab07a93..12195a08cb 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
@@ -19,11 +19,14 @@
package org.apache.gravitino.catalog.starrocks.operation;
import java.util.Collections;
+import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
+import org.apache.gravitino.catalog.jdbc.JdbcTable;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
import org.apache.gravitino.catalog.starrocks.converter.StarRocksTypeConverter;
import
org.apache.gravitino.catalog.starrocks.operations.StarRocksTableOperations;
+import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.NamedReference;
import org.apache.gravitino.rel.expressions.distributions.Distribution;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
@@ -46,15 +49,34 @@ public class TestStarRocksTableOperationsSqlGeneration {
public String createTableSql(
String tableName, JdbcColumn[] columns, Distribution distribution) {
+ return createTableSql(tableName, columns, distribution, "comment");
+ }
+
+ public String createTableSql(
+ String tableName, JdbcColumn[] columns, Distribution distribution,
String comment) {
return generateCreateTableSql(
tableName,
columns,
- "comment",
+ comment,
Collections.emptyMap(),
Transforms.EMPTY_TRANSFORM,
distribution,
Indexes.EMPTY_INDEXES);
}
+
+ public String alterTableSql(String tableName, TableChange... changes) {
+ return generateAlterTableSql("database", tableName, changes);
+ }
+
+ @Override
+ protected JdbcTable getOrCreateTable(
+ String databaseName, String tableName, JdbcTable lazyLoadCreateTable) {
+ return JdbcTable.builder()
+ .withName(tableName)
+ .withComment(
+ StringIdentifier.addToComment(StringIdentifier.fromId(42),
"existing comment"))
+ .build();
+ }
}
@Test
@@ -120,4 +142,57 @@ public class TestStarRocksTableOperationsSqlGeneration {
sql.contains("DEFAULT " +
converter.fromGravitino(col1.defaultValue())),
"Should contain DEFAULT ' ' but was: " + sql);
}
+
+ @Test
+ public void testEscapeCommentsInGeneratedSql() {
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations();
+ String tableComment = "owner\"; DROP TABLE marker; --";
+ String columnComment = "owner's comment; DROP TABLE marker; --";
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col1")
+ .withType(Types.IntegerType.get())
+ .withComment(columnComment)
+ .build();
+ Distribution distribution = Distributions.hash(1,
NamedReference.field("col1"));
+
+ String createSql =
+ ops.createTableSql("test_table", new JdbcColumn[] {column},
distribution, tableComment);
+ Assertions.assertTrue(
+ createSql.contains("COMMENT \"owner\"\"; DROP TABLE marker; -- "),
createSql);
+ Assertions.assertTrue(
+ createSql.contains("COMMENT 'owner''s comment; DROP TABLE marker;
--'"), createSql);
+
+ // The existing table's identifier (uid42) is preserved, and a sacrificial
DUMMY_ID (uid-1)
+ // marker is appended on top: the read path (extractTableCommentFromSql)
strips exactly one
+ // (outermost) marker, so the real identifier survives the round-trip.
+ String alterSql = ops.alterTableSql("test_table",
TableChange.updateComment(tableComment));
+ Assertions.assertTrue(
+ alterSql.contains(
+ "COMMENT = \"owner\"\"; DROP TABLE marker; -- "
+ + "(From Gravitino, DO NOT EDIT: gravitino.v1.uid42) "
+ + "(From Gravitino, DO NOT EDIT: gravitino.v1.uid-1)\""),
+ alterSql);
+
+ String clearCommentSql = ops.alterTableSql("test_table",
TableChange.updateComment(""));
+ Assertions.assertTrue(
+ clearCommentSql.contains(
+ "COMMENT = \"(From Gravitino, DO NOT EDIT: gravitino.v1.uid42) "
+ + "(From Gravitino, DO NOT EDIT: gravitino.v1.uid-1)\""),
+ clearCommentSql);
+ }
+
+ @Test
+ public void testEscapeAddColumnCommentInGeneratedSql() {
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations();
+ String comment = "owner\\'s \"comment\"; --";
+
+ String alterSql =
+ ops.alterTableSql(
+ "test_table",
+ TableChange.addColumn(new String[] {"col2"},
Types.IntegerType.get(), comment));
+
+ Assertions.assertTrue(alterSql.contains("ADD COLUMN `col2`"), alterSql);
+ Assertions.assertTrue(alterSql.contains("COMMENT 'owner\\\\''s
\"comment\"; --'"), alterSql);
+ }
}
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 9f77b51748..3eb449327e 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
@@ -97,6 +97,31 @@ public class TestStarRocksUtils {
assertEquals("comment", result.get("comment"));
}
+ @Test
+ public void testExtractEscapedTableCommentFromSql() {
+ String createTableSql =
+ "CREATE TABLE `testTable` (\n"
+ + "`col1` INT COMMENT \"column comment\"\n"
+ + ")\n"
+ + "COMMENT \"owner's \\\"comment\\\" (nested) C:\\\\tmp; -- "
+ + "(From Gravitino, DO NOT EDIT: gravitino.v1.uid-1)\"";
+
+ assertEquals(
+ "owner's \"comment\" (nested) C:\\tmp; --",
+ StarRocksUtils.extractTableCommentFromSql(createTableSql));
+
+ createTableSql =
+ "CREATE TABLE `testTable` (\n"
+ + "`col1` INT\n"
+ + ")\n"
+ + "COMMENT \"owner's \"\"comment\"\" D:\\\\data; -- "
+ + "(From Gravitino, DO NOT EDIT: gravitino.v1.uid-1)\"";
+
+ assertEquals(
+ "owner's \"comment\" D:\\data; --",
+ StarRocksUtils.extractTableCommentFromSql(createTableSql));
+ }
+
@Test
public void testExtractPartitionInfoFromSql() {
// test range partition
