This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 8d1c6bdb99 [Cherry-pick to branch-1.3] [#13943] fix(oceanbase): Escape
table and column comments (#13944) (#13960)
8d1c6bdb99 is described below
commit 8d1c6bdb99e9adb3c8ea3f0ebea740053621aed9
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Oct 20 11:07:07 2026 +0800
[Cherry-pick to branch-1.3] [#13943] fix(oceanbase): Escape table and
column comments (#13944) (#13960)
**Cherry-pick Information:**
- Original commit: 409d1c4013306c091c440e9bd3603c73d8b4788e
- Target branch: `branch-1.3`
- Status: ⚠️ **Has conflicts - manual resolution required**
**Do not merge** until conflict markers are resolved and the
`cherry-pick-conflict` label is removed.
Please review and resolve the conflicts before merging.
Co-authored-by: Qi Yu <[email protected]>
---
.../operation/OceanBaseTableOperations.java | 12 ++-
.../operation/TestOceanBaseTableOperations.java | 54 ++++++++++++
.../TestOceanBaseTableOperationsSqlGeneration.java | 96 ++++++++++++++++++++++
3 files changed, 158 insertions(+), 4 deletions(-)
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
index 87399f8165..f05fcfada3 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog.oceanbase.operation;
+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;
@@ -108,7 +109,7 @@ public class OceanBaseTableOperations 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
@@ -311,7 +312,7 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
newComment = StringIdentifier.addToComment(identifier, newComment);
}
}
- alterSql.add("COMMENT '" + newComment + "'");
+ alterSql.add("COMMENT '" + escapeSqlLiteral(newComment, '\'') + "'");
}
if (!setProperties.isEmpty()) {
@@ -476,7 +477,10 @@ public class OceanBaseTableOperations 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
@@ -647,7 +651,7 @@ public class OceanBaseTableOperations 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-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
index 2a063d8a94..06fc9101ad 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
@@ -45,6 +45,8 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
@Tag("gravitino-docker-test")
public class TestOceanBaseTableOperations extends TestOceanBase {
@@ -56,6 +58,58 @@ public class TestOceanBaseTableOperations extends
TestOceanBase {
DATABASE_OPERATIONS.create(TEST_DB_NAME, null, new HashMap<>());
}
+ @ParameterizedTest
+ @ValueSource(strings = {"customer's id", "quote\\' and \\new\\path\\;
\"double\"\n中文"})
+ void testCommentRoundTrips(String comment) {
+ String tableName = RandomNameUtils.genRandomName("comment_round_trip");
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col1")
+ .withType(VARCHAR)
+ .withNullable(true)
+ .withComment(comment)
+ .build();
+ try {
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME,
+ tableName,
+ new JdbcColumn[] {column},
+ comment,
+ Collections.emptyMap(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+ JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+ Assertions.assertEquals(comment, table.comment());
+ Assertions.assertEquals(comment, table.columns()[0].comment());
+
+ String updated = comment + " updated";
+ TABLE_OPERATIONS.alterTable(TEST_DB_NAME, tableName,
TableChange.updateComment(updated));
+ Assertions.assertEquals(updated, TABLE_OPERATIONS.load(TEST_DB_NAME,
tableName).comment());
+
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME, tableName, TableChange.addColumn(new String[]
{"col2"}, VARCHAR, comment));
+ table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+ Assertions.assertEquals(comment, table.columns()[1].comment());
+
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME, tableName, TableChange.updateColumnComment(new
String[] {"col1"}, updated));
+ table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+ Assertions.assertEquals(updated, table.columns()[0].comment());
+
+ // Rebuilding a column definition must also preserve an existing escaped
comment.
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ tableName,
+ TableChange.updateColumnType(new String[] {"col1"},
Types.VarCharType.of(512)));
+ table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+ Assertions.assertEquals(updated, table.columns()[0].comment());
+ Assertions.assertEquals(updated, table.comment());
+ } finally {
+ TABLE_OPERATIONS.drop(TEST_DB_NAME, tableName);
+ }
+ }
+
@Test
public void testOperationTable() {
String tableName = RandomStringUtils.randomAlphabetic(16).toLowerCase() +
"_op_table";
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperationsSqlGeneration.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperationsSqlGeneration.java
index fb15beb030..aff7fb637b 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperationsSqlGeneration.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperationsSqlGeneration.java
@@ -20,11 +20,14 @@ package org.apache.gravitino.catalog.oceanbase.operation;
import java.util.Collections;
import java.util.List;
+import java.util.stream.Stream;
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.jdbc.utils.ConnectionCountingDataSource;
import org.apache.gravitino.catalog.oceanbase.converter.OceanBaseTypeConverter;
+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;
@@ -32,6 +35,9 @@ import org.apache.gravitino.rel.indexes.Indexes;
import org.apache.gravitino.rel.types.Types;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
public class TestOceanBaseTableOperationsSqlGeneration {
@@ -59,6 +65,61 @@ public class TestOceanBaseTableOperationsSqlGeneration {
}
}
+ @ParameterizedTest
+ @MethodSource("comments")
+ void testCreateEscapesTableAndColumnComments(String comment, String escaped)
{
+ TestableOceanBaseTableOperations ops = new
TestableOceanBaseTableOperations();
+ String sql =
+ ops.generateCreateTableSql(
+ "test_table",
+ new JdbcColumn[] {column(comment)},
+ comment,
+ Collections.emptyMap(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+
+ Assertions.assertTrue(sql.contains("COMMENT '" + escaped + "' "), sql);
+ Assertions.assertTrue(sql.endsWith("COMMENT='" + escaped + "';"), sql);
+ }
+
+ @ParameterizedTest
+ @MethodSource("comments")
+ void testUpdateTableCommentEscapesLiteral(String comment, String escaped) {
+ TestableOceanBaseTableOperations ops = operationsWithTable();
+ String sql =
+ ops.generateAlterTableSql("database", "test_table",
TableChange.updateComment(comment));
+
+ Assertions.assertEquals("ALTER TABLE `test_table`\nCOMMENT '" + escaped +
"';", sql);
+ }
+
+ @ParameterizedTest
+ @MethodSource("comments")
+ void testAddColumnEscapesComment(String comment, String escaped) {
+ TestableOceanBaseTableOperations ops = operationsWithTable();
+ String sql =
+ ops.generateAlterTableSql(
+ "database",
+ "test_table",
+ TableChange.addColumn(new String[] {"new_col"},
Types.IntegerType.get(), comment));
+
+ Assertions.assertTrue(sql.contains("COMMENT '" + escaped + "' "), sql);
+ }
+
+ @ParameterizedTest
+ @MethodSource("comments")
+ void testModifyColumnEscapesComment(String comment, String escaped) {
+ TestableOceanBaseTableOperations ops = operationsWithTable();
+ String sql =
+ ops.generateAlterTableSql(
+ "database",
+ "test_table",
+ TableChange.updateColumnComment(new String[] {"col1"}, comment));
+
+ Assertions.assertTrue(sql.contains("MODIFY COLUMN `col1`"), sql);
+ Assertions.assertTrue(sql.contains("COMMENT '" + escaped + "' "), sql);
+ }
+
@Test
void testPurgeIsRejectedWithoutExecutingSql() {
ConnectionCountingDataSource connections = new
ConnectionCountingDataSource();
@@ -124,4 +185,39 @@ public class TestOceanBaseTableOperationsSqlGeneration {
sql.contains("DEFAULT " +
converter.fromGravitino(col1.defaultValue())),
"Should contain DEFAULT value but was: " + sql);
}
+
+ private static JdbcColumn column(String comment) {
+ return JdbcColumn.builder()
+ .withName("col1")
+ .withType(Types.IntegerType.get())
+ .withNullable(true)
+ .withComment(comment)
+ .build();
+ }
+
+ private static TestableOceanBaseTableOperations operationsWithTable() {
+ JdbcTable table =
+ JdbcTable.builder()
+ .withName("test_table")
+ .withComment("old comment")
+ .withColumns(new JdbcColumn[] {column("old comment")})
+ .build();
+ return new TestableOceanBaseTableOperations() {
+ /** Returns the table fixture without accessing a database. */
+ @Override
+ public JdbcTable load(String databaseName, String tableName) {
+ Assertions.assertEquals("database", databaseName);
+ Assertions.assertEquals("test_table", tableName);
+ return table;
+ }
+ };
+ }
+
+ private static Stream<Arguments> comments() {
+ return Stream.of(
+ Arguments.of("customer's id", "customer''s id"),
+ Arguments.of("C:\\new\\trailing\\", "C:\\\\new\\\\trailing\\\\"),
+ Arguments.of("backslash\\'quote; --", "backslash\\\\''quote; --"),
+ Arguments.of("line1\n中文 \"double\"", "line1\n中文 \"double\""));
+ }
}
