This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 409d1c4013 [#13943] fix(oceanbase): Escape table and column comments
(#13944)
409d1c4013 is described below
commit 409d1c4013306c091c440e9bd3603c73d8b4788e
Author: Qi Yu <[email protected]>
AuthorDate: Mon Oct 19 22:05:59 2026 +0800
[#13943] fix(oceanbase): Escape table and column comments (#13944)
### What changes were proposed in this pull request?
Use the existing `escapeSqlLiteral` helper for all four OceanBase
comment SQL-generation paths: create table, update table comment, add
column, and create/modify column definitions.
Add SQL-generation regression tests and real OceanBase round-trip tests,
including changes that rebuild a column definition while preserving its
existing comment.
### Why are the changes needed?
Unescaped apostrophes break comment DDL, and backslash escapes can
change the stored text. Apply the same literal escaping already used by
the MySQL catalog.
This follows the existing MySQL-compatible literal semantics with
backslash escaping enabled. SQL-mode-aware escaping for
`NO_BACKSLASH_ESCAPES` is outside this patch.
Fix: #13943
### Does this PR introduce _any_ user-facing change?
OceanBase table and column comments containing quotes or backslashes can
be created and altered without corrupting their text. No API or
configuration changes.
### How was this patch tested?
- OceanBase catalog non-Docker unit tests: 25 passed, including 16 new
SQL-generation cases.
- OceanBase catalog tests with Docker enabled: 43 passed, including two
real comment round-trip cases on OceanBase CE 4.2.1.10.
- Actual SQL mode:
`STRICT_ALL_TABLES,NO_ZERO_IN_DATE,NO_AUTO_CREATE_USER`.
- Mutation check: reverting the production fix makes 12 of the 16 new
SQL-generation cases fail; the four cases requiring no escaping still
pass.
- OceanBase catalog Spotless checks.
---
.../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 312e6d7a1e..687cf5f19d 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
@@ -314,7 +315,7 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
newComment = StringIdentifier.addToComment(identifier, newComment);
}
}
- alterSql.add("COMMENT '" + newComment + "'");
+ alterSql.add("COMMENT '" + escapeSqlLiteral(newComment, '\'') + "'");
}
if (!setProperties.isEmpty()) {
@@ -479,7 +480,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
@@ -650,7 +654,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 74ea40b2d5..612c034022 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();
@@ -123,4 +184,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\""));
+ }
}
