This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch branch-0.9
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-0.9 by this push:
new e096d09b46 improvement(catalogs): fix index existence check (#7660)
e096d09b46 is described below
commit e096d09b463170a3ae34d4e73a3ae2b7dddde731
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Jul 11 10:04:34 2025 +0800
improvement(catalogs): fix index existence check (#7660)
### What changes were proposed in this pull request?
fix index existence check
### Why are the changes needed?
Fix: #7591
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
tests added.
Co-authored-by: BIN <[email protected]>
Co-authored-by: senlizishi <[email protected]>
---
.../doris/operation/DorisTableOperations.java | 2 +-
.../doris/operation/TestDorisTableOperations.java | 48 +++++++++++++++++++
.../mysql/operation/MysqlTableOperations.java | 10 ++--
.../mysql/operation/TestMysqlTableOperations.java | 54 +++++++++++++++++++++-
.../operation/OceanBaseTableOperations.java | 10 ++--
.../operation/TestOceanBaseTableOperations.java | 53 ++++++++++++++++++++-
6 files changed, 162 insertions(+), 15 deletions(-)
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 425bf51d4c..48f48575f3 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
@@ -774,7 +774,7 @@ public class DorisTableOperations extends
JdbcTableOperations {
static String deleteIndexDefinition(
JdbcTable lazyLoadTable, TableChange.DeleteIndex deleteIndex) {
- if (deleteIndex.isIfExists()) {
+ if (!deleteIndex.isIfExists()) {
Preconditions.checkArgument(
Arrays.stream(lazyLoadTable.index())
.anyMatch(index -> index.name().equals(deleteIndex.getName())),
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java
index a9dfb0ed60..dd164473b4 100644
---
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java
@@ -640,4 +640,52 @@ public class TestDorisTableOperations extends TestDoris {
assertTrue(loadedListPartitions.containsKey("p2"));
assertTrue(Arrays.deepEquals(listPartition2.lists(),
loadedListPartitions.get("p2").lists()));
}
+
+ @Test
+ public void testOperationIndexDefinition() {
+ String tableName =
GravitinoITUtils.genRandomName("doris_basic_test_table");
+ String tableComment = "test_comment";
+ List<JdbcColumn> columns = new ArrayList<>();
+ JdbcColumn col_1 =
+
JdbcColumn.builder().withName("col_1").withType(INT).withComment("id").build();
+ columns.add(col_1);
+ JdbcColumn col_2 =
+
JdbcColumn.builder().withName("col_2").withType(VARCHAR_255).withComment("col_2").build();
+ columns.add(col_2);
+
+ Distribution distribution =
+ Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("col_1"));
+ Index[] indexes = new Index[] {Indexes.unique("uk_2", new String[][]
{{"col_1"}})};
+
+ // create table
+ TABLE_OPERATIONS.create(
+ databaseName,
+ tableName,
+ columns.toArray(new JdbcColumn[0]),
+ tableComment,
+ createProperties(),
+ null,
+ distribution,
+ indexes);
+ JdbcTable load = TABLE_OPERATIONS.load(databaseName, tableName);
+
+ // If ifExists is set to true then the code should not throw an exception
if the index doesn't
+ // exist.
+ TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1",
true);
+ String sql = DorisTableOperations.deleteIndexDefinition(null, deleteIndex);
+ Assertions.assertEquals("DROP INDEX uk_1", sql);
+
+ // The index existence check should only verify existence when ifExists is
false, preventing
+ // failures when dropping non-existent indexes.
+ TableChange.DeleteIndex deleteIndex2 = new TableChange.DeleteIndex("uk_1",
false);
+ IllegalArgumentException thrown =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> DorisTableOperations.deleteIndexDefinition(load,
deleteIndex2));
+ Assertions.assertEquals("Index does not exist", thrown.getMessage());
+
+ TableChange.DeleteIndex deleteIndex3 = new TableChange.DeleteIndex("uk_2",
false);
+ sql = DorisTableOperations.deleteIndexDefinition(load, deleteIndex3);
+ Assertions.assertEquals("DROP INDEX uk_2", sql);
+ }
}
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 36b4daebf9..35683e7346 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
@@ -346,11 +346,11 @@ public class MysqlTableOperations extends
JdbcTableOperations {
@VisibleForTesting
static String deleteIndexDefinition(
JdbcTable lazyLoadTable, TableChange.DeleteIndex deleteIndex) {
- if (deleteIndex.isIfExists()) {
- if (Arrays.stream(lazyLoadTable.index())
- .anyMatch(index -> index.name().equals(deleteIndex.getName()))) {
- throw new IllegalArgumentException("Index does not exist");
- }
+ if (!deleteIndex.isIfExists()) {
+ Preconditions.checkArgument(
+ Arrays.stream(lazyLoadTable.index())
+ .anyMatch(index -> index.name().equals(deleteIndex.getName())),
+ "Index does not exist");
}
return "DROP INDEX " + BACK_QUOTE + deleteIndex.getName() + BACK_QUOTE;
}
diff --git
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
index 923e20fa0c..e33533742b 100644
---
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
+++
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
@@ -1051,8 +1051,58 @@ public class TestMysqlTableOperations extends TestMysql {
sql = MysqlTableOperations.addIndexDefinition(successIndex);
Assertions.assertEquals("ADD PRIMARY KEY (`col_1`, `col_2`)", sql);
- TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1",
false);
- sql = MysqlTableOperations.deleteIndexDefinition(null, deleteIndex);
+ String tableName = RandomStringUtils.randomAlphabetic(16) + "_op_table";
+ String tableComment = "test_comment";
+ List<JdbcColumn> columns = new ArrayList<>();
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(INT)
+ .withComment("id")
+ .withNullable(false)
+ .build());
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_2")
+ .withType(INT)
+ .withNullable(true)
+ .withDefaultValue(Literals.NULL)
+ .build());
+ Map<String, String> properties = new HashMap<>();
+ properties.put(MYSQL_AUTO_INCREMENT_OFFSET_KEY, "10");
+
+ Index[] indexes = new Index[] {Indexes.unique("uk_2", new String[][]
{{"col_1"}})};
+ // create table
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME.toString(),
+ tableName,
+ columns.toArray(new JdbcColumn[0]),
+ tableComment,
+ properties,
+ null,
+ Distributions.NONE,
+ indexes);
+
+ // load table
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName);
+
+ // If ifExists is set to true then the code should not throw an exception
if the index doesn't
+ // exist.
+ TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1",
true);
+ sql = MysqlTableOperations.deleteIndexDefinition(load, deleteIndex);
Assertions.assertEquals("DROP INDEX `uk_1`", sql);
+
+ // The index existence check should only verify existence when ifExists is
false, preventing
+ // failures when dropping non-existent indexes.
+ TableChange.DeleteIndex deleteIndex2 = new TableChange.DeleteIndex("uk_1",
false);
+ IllegalArgumentException thrown =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> MysqlTableOperations.deleteIndexDefinition(load,
deleteIndex2));
+ Assertions.assertEquals("Index does not exist", thrown.getMessage());
+
+ TableChange.DeleteIndex deleteIndex3 = new TableChange.DeleteIndex("uk_2",
false);
+ sql = MysqlTableOperations.deleteIndexDefinition(load, deleteIndex3);
+ Assertions.assertEquals("DROP INDEX `uk_2`", sql);
}
}
diff --git
a/catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
b/catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
index 98f2d174f1..e79250b1b1 100644
---
a/catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
+++
b/catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
@@ -361,11 +361,11 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
@VisibleForTesting
static String deleteIndexDefinition(
JdbcTable lazyLoadTable, TableChange.DeleteIndex deleteIndex) {
- if (deleteIndex.isIfExists()) {
- if (Arrays.stream(lazyLoadTable.index())
- .anyMatch(index -> index.name().equals(deleteIndex.getName()))) {
- throw new IllegalArgumentException("Index does not exist");
- }
+ if (!deleteIndex.isIfExists()) {
+ Preconditions.checkArgument(
+ Arrays.stream(lazyLoadTable.index())
+ .anyMatch(index -> index.name().equals(deleteIndex.getName())),
+ "Index does not exist");
}
return "DROP INDEX " + BACK_QUOTE + deleteIndex.getName() + BACK_QUOTE;
}
diff --git
a/catalogs/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
b/catalogs/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
index 6f2a422fbd..c8d4fb2382 100644
---
a/catalogs/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
+++
b/catalogs/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
@@ -990,8 +990,57 @@ public class TestOceanBaseTableOperations extends
TestOceanBase {
sql = OceanBaseTableOperations.addIndexDefinition(successIndex);
Assertions.assertEquals("ADD PRIMARY KEY (`col_1`, `col_2`)", sql);
- TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1",
false);
- sql = OceanBaseTableOperations.deleteIndexDefinition(null, deleteIndex);
+ String tableName = RandomStringUtils.randomAlphabetic(16).toLowerCase() +
"_oi_table";
+ String tableComment = "test_comment";
+ List<JdbcColumn> columns = new ArrayList<>();
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(INT)
+ .withNullable(false)
+ .withComment("set primary key")
+ .build());
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_2")
+ .withType(VARCHAR)
+ .withComment("test_comment")
+ .withNullable(true)
+ .build());
+ Map<String, String> properties = new HashMap<>();
+
+ Index[] indexes = new Index[] {Indexes.unique("uk_2", new String[][]
{{"col_1"}})};
+ // create table
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME,
+ tableName,
+ columns.toArray(new JdbcColumn[0]),
+ tableComment,
+ properties,
+ null,
+ Distributions.NONE,
+ indexes);
+
+ // load table
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+
+ // If ifExists is set to true then the code should not throw an exception
if the index doesn't
+ // exist.
+ TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1",
true);
+ sql = OceanBaseTableOperations.deleteIndexDefinition(load, deleteIndex);
Assertions.assertEquals("DROP INDEX `uk_1`", sql);
+
+ // The index existence check should only verify existence when ifExists is
false, preventing
+ // failures when dropping non-existent indexes.
+ TableChange.DeleteIndex deleteIndex2 = new TableChange.DeleteIndex("uk_1",
false);
+ IllegalArgumentException thrown =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> OceanBaseTableOperations.deleteIndexDefinition(load,
deleteIndex2));
+ Assertions.assertEquals("Index does not exist", thrown.getMessage());
+
+ TableChange.DeleteIndex deleteIndex3 = new TableChange.DeleteIndex("uk_2",
false);
+ sql = OceanBaseTableOperations.deleteIndexDefinition(load, deleteIndex3);
+ Assertions.assertEquals("DROP INDEX `uk_2`", sql);
}
}
