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 1762bf6be9 [Cherry-pick to branch-1.3] [#13916] fix(jdbc): reject
unsupported StarRocks and OceanBase purge (#13917) (#13947)
1762bf6be9 is described below
commit 1762bf6be93f00f90b8f9892cc8e0d857573f11a
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Oct 20 08:20:32 2026 +0800
[Cherry-pick to branch-1.3] [#13916] fix(jdbc): reject unsupported
StarRocks and OceanBase purge (#13917) (#13947)
**Cherry-pick Information:**
- Original commit: 6968f0b0baf258eeeaa8b29c17c91892657131b0
- Target branch: `branch-1.3`
- Status: Clean cherry-pick with the test utility and purge-order
prerequisite included for branch-1.3.
### Backport prerequisites
- Include the shared `ConnectionCountingDataSource` test utility
referenced by StarRocks and OceanBase tests.
- Generate purge SQL before acquiring the execution connection, so
unsupported operations fail without borrowing a connection.
- Assert total connection borrows are zero for rejected purge requests,
in addition to checking no SQL executed and no connections leaked.
### Validation
- JDBC common, StarRocks, and OceanBase non-Docker suites: 60, 13, and 6
tests passed.
- Spotless checks passed.
- Reverting only the purge-order fix makes both zero-borrow regression
tests fail.
---------
Co-authored-by: Qi Yu <[email protected]>
---
.../operation/OceanBaseTableOperations.java | 3 +-
.../integration/test/CatalogOceanBaseIT.java | 6 +-
.../TestOceanBaseTableOperationsSqlGeneration.java | 35 +++++
.../jdbc/operation/JdbcTableOperations.java | 4 +-
.../jdbc/utils/ConnectionCountingDataSource.java | 168 +++++++++++++++++++++
.../operations/StarRocksTableOperations.java | 6 -
.../TestStarRocksTableOperationsSqlGeneration.java | 35 +++++
docs/jdbc-oceanbase-catalog.md | 7 +
docs/jdbc-starrocks-catalog.md | 7 +
9 files changed, 262 insertions(+), 9 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 aa9a49cf53..87399f8165 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
@@ -196,7 +196,8 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
@Override
protected String generatePurgeTableSql(String tableName) {
- return String.format("TRUNCATE TABLE `%s`", tableName);
+ throw new UnsupportedOperationException(
+ "OceanBase does not support purge table in Gravitino, please use drop
table");
}
/**
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/integration/test/CatalogOceanBaseIT.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/integration/test/CatalogOceanBaseIT.java
index 260d851ac7..63532f42c3 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/integration/test/CatalogOceanBaseIT.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/integration/test/CatalogOceanBaseIT.java
@@ -1460,9 +1460,13 @@ public class CatalogOceanBaseIT extends BaseIT {
catalog.asTableCatalog().listTables(Namespace.of(testSchemaName));
Assertions.assertTrue(Arrays.stream(tableIdents).anyMatch(t ->
t.name().equals(testTableName)));
+ Assertions.assertThrows(
+ UnsupportedOperationException.class, () ->
catalog.asTableCatalog().purgeTable(tableIdent));
+ Assertions.assertTrue(catalog.asTableCatalog().tableExists(tableIdent));
Assertions.assertTrue(catalog.asTableCatalog().dropTable(tableIdent));
Assertions.assertFalse(catalog.asTableCatalog().tableExists(tableIdent));
- Assertions.assertFalse(catalog.asTableCatalog().purgeTable(tableIdent));
+ Assertions.assertThrows(
+ UnsupportedOperationException.class, () ->
catalog.asTableCatalog().purgeTable(tableIdent));
catalog.asSchemas().dropSchema(testSchemaName, true);
// sql injection
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 2897660bfc..fb15beb030 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
@@ -19,9 +19,11 @@
package org.apache.gravitino.catalog.oceanbase.operation;
import java.util.Collections;
+import java.util.List;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
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.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.literals.Literals;
@@ -40,6 +42,11 @@ public class TestOceanBaseTableOperationsSqlGeneration {
super.columnDefaultValueConverter = new
JdbcColumnDefaultValueConverter();
}
+ private TestableOceanBaseTableOperations(ConnectionCountingDataSource
connections) {
+ this();
+ dataSource = connections.dataSource();
+ }
+
public String createTableSql(String tableName, JdbcColumn[] columns) {
return generateCreateTableSql(
tableName,
@@ -52,6 +59,34 @@ public class TestOceanBaseTableOperationsSqlGeneration {
}
}
+ @Test
+ void testPurgeIsRejectedWithoutExecutingSql() {
+ ConnectionCountingDataSource connections = new
ConnectionCountingDataSource();
+ TestableOceanBaseTableOperations ops = new
TestableOceanBaseTableOperations(connections);
+
+ UnsupportedOperationException exception =
+ Assertions.assertThrows(
+ UnsupportedOperationException.class, () -> ops.purge("database",
"test_table"));
+
+ Assertions.assertEquals(
+ "OceanBase does not support purge table in Gravitino, please use drop
table",
+ exception.getMessage());
+ Assertions.assertTrue(connections.executedSql().isEmpty());
+ Assertions.assertEquals(0, connections.totalBorrows());
+ Assertions.assertEquals(0, connections.borrowed());
+ }
+
+ @Test
+ void testDropStillExecutesDropTable() {
+ ConnectionCountingDataSource connections = new
ConnectionCountingDataSource();
+ TestableOceanBaseTableOperations ops = new
TestableOceanBaseTableOperations(connections);
+
+ Assertions.assertTrue(ops.drop("database", "test_table"));
+
+ Assertions.assertEquals(List.of("DROP TABLE `test_table`"),
connections.executedSql());
+ Assertions.assertEquals(0, connections.borrowed());
+ }
+
@Test
public void testCreateTableWithEmptyStringDefaultValue() {
TestableOceanBaseTableOperations ops = new
TestableOceanBaseTableOperations();
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java
index 73ba6fe1b6..7f9b9cfa14 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java
@@ -379,8 +379,10 @@ public abstract class JdbcTableOperations implements
TableOperation {
protected void purgeTable(String databaseName, String tableName) {
LOG.info("Attempting to purge table {} from database {}", tableName,
databaseName);
+ // Reject unsupported purge operations before borrowing an execution
connection.
+ String sql = generatePurgeTableSql(databaseName, tableName);
try (Connection connection = getConnection(databaseName)) {
- JdbcConnectorUtils.executeUpdate(connection,
generatePurgeTableSql(databaseName, tableName));
+ JdbcConnectorUtils.executeUpdate(connection, sql);
LOG.info("Purge table {} from database {}", tableName, databaseName);
} catch (final SQLException se) {
throw this.exceptionMapper.toGravitinoException(se);
diff --git
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/ConnectionCountingDataSource.java
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/ConnectionCountingDataSource.java
new file mode 100644
index 0000000000..e2a947745b
--- /dev/null
+++
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/ConnectionCountingDataSource.java
@@ -0,0 +1,168 @@
+/*
+ * 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 java.lang.reflect.Proxy;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.Statement;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import javax.sql.DataSource;
+
+/**
+ * A test {@link DataSource} whose connections only record the SQL they
execute. It tracks how many
+ * connections are borrowed at once, so tests can assert that an operation
never holds one
+ * connection while borrowing another.
+ */
+public class ConnectionCountingDataSource {
+
+ private final DatabaseMetaData metadata;
+
+ private final AtomicInteger borrowed = new AtomicInteger();
+ private final AtomicInteger peakBorrowed = new AtomicInteger();
+ private final AtomicInteger totalBorrows = new AtomicInteger();
+ private final List<String> executedSql = new CopyOnWriteArrayList<>();
+ private final DataSource dataSource =
+ (DataSource)
+ Proxy.newProxyInstance(
+ DataSource.class.getClassLoader(),
+ new Class<?>[] {DataSource.class},
+ (proxy, method, args) -> {
+ if ("getConnection".equals(method.getName())) {
+ return borrow();
+ }
+ return objectMethod(proxy, method.getName(), args,
"DataSource");
+ });
+
+ /** Creates a counting data source whose driver reports a null version. */
+ public ConnectionCountingDataSource() {
+ this(metaData());
+ }
+
+ /**
+ * Creates a counting data source with the supplied JDBC metadata.
+ *
+ * @param metadata the metadata returned by each borrowed connection
+ */
+ public ConnectionCountingDataSource(DatabaseMetaData metadata) {
+ this.metadata = metadata;
+ }
+
+ /** Returns the data source to pass to the operations under test. */
+ public DataSource dataSource() {
+ return dataSource;
+ }
+
+ /** Returns the number of connections currently borrowed and not yet closed.
*/
+ public int borrowed() {
+ return borrowed.get();
+ }
+
+ /** Returns the largest number of connections that were borrowed at the same
time. */
+ public int peakBorrowed() {
+ return peakBorrowed.get();
+ }
+
+ /** Returns how many connections were borrowed in total. */
+ public int totalBorrows() {
+ return totalBorrows.get();
+ }
+
+ /** Returns the update statements executed so far, in order. */
+ public List<String> executedSql() {
+ return executedSql;
+ }
+
+ private Connection borrow() {
+ totalBorrows.incrementAndGet();
+ peakBorrowed.accumulateAndGet(borrowed.incrementAndGet(), Math::max);
+ AtomicBoolean closed = new AtomicBoolean();
+ return (Connection)
+ Proxy.newProxyInstance(
+ Connection.class.getClassLoader(),
+ new Class<?>[] {Connection.class},
+ (proxy, method, args) -> {
+ switch (method.getName()) {
+ case "close":
+ if (closed.compareAndSet(false, true)) {
+ borrowed.decrementAndGet();
+ }
+ return null;
+ case "setCatalog":
+ case "setSchema":
+ return null;
+ case "createStatement":
+ return statement();
+ case "getMetaData":
+ return metadata;
+ default:
+ return objectMethod(proxy, method.getName(), args,
"Connection");
+ }
+ });
+ }
+
+ // Answers Object methods so the proxies can be printed or compared, and
rejects anything else.
+ private static Object objectMethod(Object proxy, String name, Object[] args,
String type) {
+ switch (name) {
+ case "toString":
+ return "Counting" + type + "@" +
Integer.toHexString(System.identityHashCode(proxy));
+ case "hashCode":
+ return System.identityHashCode(proxy);
+ case "equals":
+ return proxy == args[0];
+ default:
+ throw new UnsupportedOperationException(type + "." + name);
+ }
+ }
+
+ // Reports a null driver version, which the JDBC contract allows.
+ private static DatabaseMetaData metaData() {
+ return (DatabaseMetaData)
+ Proxy.newProxyInstance(
+ DatabaseMetaData.class.getClassLoader(),
+ new Class<?>[] {DatabaseMetaData.class},
+ (proxy, method, args) -> {
+ if ("getDriverVersion".equals(method.getName())) {
+ return null;
+ }
+ return objectMethod(proxy, method.getName(), args,
"DatabaseMetaData");
+ });
+ }
+
+ private Statement statement() {
+ return (Statement)
+ Proxy.newProxyInstance(
+ Statement.class.getClassLoader(),
+ new Class<?>[] {Statement.class},
+ (proxy, method, args) -> {
+ switch (method.getName()) {
+ case "executeUpdate":
+ executedSql.add((String) args[0]);
+ return 0;
+ case "close":
+ return null;
+ default:
+ return objectMethod(proxy, method.getName(), args,
"Statement");
+ }
+ });
+ }
+}
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 8d6af056fc..439deee4dd 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
@@ -306,16 +306,10 @@ public class StarRocksTableOperations extends
JdbcTableOperations {
@Override
protected String generatePurgeTableSql(String tableName) {
- // never called, as implemented generatePurgeTableSql(String databaseName,
String tableName)
throw new UnsupportedOperationException(
"StarRocks does not support purge table in Gravitino, please use drop
table");
}
- @Override
- protected String generatePurgeTableSql(String databaseName, String
tableName) {
- return String.format("TRUNCATE TABLE `%s`.`%s`", databaseName, tableName);
- }
-
@Override
protected Distribution getDistributionInfo(
Connection connection, String databaseName, String tableName) throws
SQLException {
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 b16e0e96b4..c6e3662a5d 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
@@ -20,11 +20,13 @@ package org.apache.gravitino.catalog.starrocks.operation;
import java.sql.Connection;
import java.util.Collections;
+import java.util.List;
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.jdbc.utils.ConnectionCountingDataSource;
import org.apache.gravitino.catalog.starrocks.converter.StarRocksTypeConverter;
import
org.apache.gravitino.catalog.starrocks.operations.StarRocksTableOperations;
import org.apache.gravitino.rel.TableChange;
@@ -53,6 +55,11 @@ public class TestStarRocksTableOperationsSqlGeneration {
super.columnDefaultValueConverter = new
JdbcColumnDefaultValueConverter();
}
+ private TestableStarRocksTableOperations(ConnectionCountingDataSource
connections) {
+ this();
+ dataSource = connections.dataSource();
+ }
+
public String createTableSql(
String tableName, JdbcColumn[] columns, Distribution distribution) {
return createTableSql(tableName, columns, distribution, "comment");
@@ -143,6 +150,34 @@ public class TestStarRocksTableOperationsSqlGeneration {
"test_table", new JdbcColumn[] {column}, distribution, "comment",
indexes));
}
+ @Test
+ void testPurgeIsRejectedWithoutExecutingSql() {
+ ConnectionCountingDataSource connections = new
ConnectionCountingDataSource();
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations(connections);
+
+ UnsupportedOperationException exception =
+ Assertions.assertThrows(
+ UnsupportedOperationException.class, () -> ops.purge("database",
"test_table"));
+
+ Assertions.assertEquals(
+ "StarRocks does not support purge table in Gravitino, please use drop
table",
+ exception.getMessage());
+ Assertions.assertTrue(connections.executedSql().isEmpty());
+ Assertions.assertEquals(0, connections.totalBorrows());
+ Assertions.assertEquals(0, connections.borrowed());
+ }
+
+ @Test
+ void testDropStillExecutesDropTable() {
+ ConnectionCountingDataSource connections = new
ConnectionCountingDataSource();
+ TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations(connections);
+
+ Assertions.assertTrue(ops.drop("database", "test_table"));
+
+ Assertions.assertEquals(List.of("DROP TABLE `test_table`"),
connections.executedSql());
+ Assertions.assertEquals(0, connections.borrowed());
+ }
+
@Test
public void testCreateTableWithEmptyStringDefaultValue() {
TestableStarRocksTableOperations ops = new
TestableStarRocksTableOperations();
diff --git a/docs/jdbc-oceanbase-catalog.md b/docs/jdbc-oceanbase-catalog.md
index 22ffc1c0ec..9e0b99e7a5 100644
--- a/docs/jdbc-oceanbase-catalog.md
+++ b/docs/jdbc-oceanbase-catalog.md
@@ -260,6 +260,13 @@ The OceanBase catalog does not support creating
partitioned tables in the curren
Refer to [Manage Relational Metadata Using
Gravitino](./manage-relational-metadata-using-gravitino.md#table-operations)
for more details.
+#### Drop and Purge Table
+
+- Dropping a table runs `DROP TABLE` in OceanBase. Whether OceanBase keeps the
dropped table in its
+ recycle bin depends on the OceanBase configuration.
+- Purging a table is not supported. A purge request fails with an
unsupported-operation error, and
+ the table and its data stay unchanged. Use drop instead.
+
#### Alter Table Operations
Gravitino supports these table alteration operations:
diff --git a/docs/jdbc-starrocks-catalog.md b/docs/jdbc-starrocks-catalog.md
index 26839c4d33..a03d268cbe 100644
--- a/docs/jdbc-starrocks-catalog.md
+++ b/docs/jdbc-starrocks-catalog.md
@@ -177,6 +177,13 @@ For the `RANDOM` distribution strategy, Gravitino uses the
`EVEN` to represent i
Refer to [Manage Relational Metadata Using
Gravitino](./manage-relational-metadata-using-gravitino.md#table-operations)
for more details.
+#### Drop and Purge Table
+
+- Dropping a table runs `DROP TABLE` in StarRocks. Whether StarRocks keeps the
dropped table in its
+ recycle bin depends on the StarRocks configuration.
+- Purging a table is not supported. A purge request fails with an
unsupported-operation error, and
+ the table and its data stay unchanged. Use drop instead.
+
#### Alter Table Operations
Gravitino supports these table alteration operations:
