This is an automated email from the ASF dual-hosted git repository.
diqiu50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new afbd958c45 [MINOR] fix(jdbc): Quote comments in generated DDL (#12007)
afbd958c45 is described below
commit afbd958c45c3007d7324a69e402a9d126ad3d361
Author: Qi Yu <[email protected]>
AuthorDate: Tue Jul 21 16:38:31 2026 +0800
[MINOR] fix(jdbc): Quote comments in generated DDL (#12007)
### What changes were proposed in this pull request?
- Add a shared utility for escaping SQL string literal content.
- Escape table, column, and schema comments in generated JDBC DDL.
- Apply the escaping to MySQL, PostgreSQL, Doris, and StarRocks
operations.
- Add regression tests covering quotes, backslashes, semicolons, and SQL
comment tokens.
### Why are the changes needed?
JDBC catalog comments were inserted directly into generated DDL
statements. Comments containing quotes could produce malformed SQL or
change the structure of the generated statement.
The change ensures comment content remains inside its SQL string
literal.
### Does this PR introduce _any_ user-facing change?
Yes. JDBC table, column, and PostgreSQL schema comments containing
quotes or backslashes are now handled correctly.
There is no API or configuration change.
### How was this patch tested?
- `./gradlew spotlessApply`
- `env SKIP_DOCKER_TESTS=true ./gradlew
:catalogs:catalog-jdbc-common:test :catalogs:catalog-jdbc-mysql:test
:catalogs:catalog-jdbc-postgresql:test :catalogs:catalog-jdbc-doris:test
:catalogs:catalog-jdbc-starrocks:test -PskipITs -PskipDockerTests=true`
---
.../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
