Skip to content

Commit e096d09

Browse files
github-actions[bot]senlizishisenlizishi
authored
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]>
1 parent 00d2dd0 commit e096d09

File tree

6 files changed

+162
-15
lines changed

6 files changed

+162
-15
lines changed

‎catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,7 @@ static String addIndexDefinition(TableChange.AddIndex addIndex) {
774774

775775
static String deleteIndexDefinition(
776776
JdbcTable lazyLoadTable, TableChange.DeleteIndex deleteIndex) {
777-
if (deleteIndex.isIfExists()) {
777+
if (!deleteIndex.isIfExists()) {
778778
Preconditions.checkArgument(
779779
Arrays.stream(lazyLoadTable.index())
780780
.anyMatch(index -> index.name().equals(deleteIndex.getName())),

‎catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperations.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,4 +640,52 @@ public void testCreatePartitionedTable() {
640640
assertTrue(loadedListPartitions.containsKey("p2"));
641641
assertTrue(Arrays.deepEquals(listPartition2.lists(), loadedListPartitions.get("p2").lists()));
642642
}
643+
644+
@Test
645+
public void testOperationIndexDefinition() {
646+
String tableName = GravitinoITUtils.genRandomName("doris_basic_test_table");
647+
String tableComment = "test_comment";
648+
List<JdbcColumn> columns = new ArrayList<>();
649+
JdbcColumn col_1 =
650+
JdbcColumn.builder().withName("col_1").withType(INT).withComment("id").build();
651+
columns.add(col_1);
652+
JdbcColumn col_2 =
653+
JdbcColumn.builder().withName("col_2").withType(VARCHAR_255).withComment("col_2").build();
654+
columns.add(col_2);
655+
656+
Distribution distribution =
657+
Distributions.hash(DEFAULT_BUCKET_SIZE, NamedReference.field("col_1"));
658+
Index[] indexes = new Index[] {Indexes.unique("uk_2", new String[][] {{"col_1"}})};
659+
660+
// create table
661+
TABLE_OPERATIONS.create(
662+
databaseName,
663+
tableName,
664+
columns.toArray(new JdbcColumn[0]),
665+
tableComment,
666+
createProperties(),
667+
null,
668+
distribution,
669+
indexes);
670+
JdbcTable load = TABLE_OPERATIONS.load(databaseName, tableName);
671+
672+
// If ifExists is set to true then the code should not throw an exception if the index doesn't
673+
// exist.
674+
TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1", true);
675+
String sql = DorisTableOperations.deleteIndexDefinition(null, deleteIndex);
676+
Assertions.assertEquals("DROP INDEX uk_1", sql);
677+
678+
// The index existence check should only verify existence when ifExists is false, preventing
679+
// failures when dropping non-existent indexes.
680+
TableChange.DeleteIndex deleteIndex2 = new TableChange.DeleteIndex("uk_1", false);
681+
IllegalArgumentException thrown =
682+
Assertions.assertThrows(
683+
IllegalArgumentException.class,
684+
() -> DorisTableOperations.deleteIndexDefinition(load, deleteIndex2));
685+
Assertions.assertEquals("Index does not exist", thrown.getMessage());
686+
687+
TableChange.DeleteIndex deleteIndex3 = new TableChange.DeleteIndex("uk_2", false);
688+
sql = DorisTableOperations.deleteIndexDefinition(load, deleteIndex3);
689+
Assertions.assertEquals("DROP INDEX uk_2", sql);
690+
}
643691
}

‎catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -346,11 +346,11 @@ private String updateColumnAutoIncrementDefinition(
346346
@VisibleForTesting
347347
static String deleteIndexDefinition(
348348
JdbcTable lazyLoadTable, TableChange.DeleteIndex deleteIndex) {
349-
if (deleteIndex.isIfExists()) {
350-
if (Arrays.stream(lazyLoadTable.index())
351-
.anyMatch(index -> index.name().equals(deleteIndex.getName()))) {
352-
throw new IllegalArgumentException("Index does not exist");
353-
}
349+
if (!deleteIndex.isIfExists()) {
350+
Preconditions.checkArgument(
351+
Arrays.stream(lazyLoadTable.index())
352+
.anyMatch(index -> index.name().equals(deleteIndex.getName())),
353+
"Index does not exist");
354354
}
355355
return "DROP INDEX " + BACK_QUOTE + deleteIndex.getName() + BACK_QUOTE;
356356
}

‎catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,8 +1051,58 @@ public void testOperationIndexDefinition() {
10511051
sql = MysqlTableOperations.addIndexDefinition(successIndex);
10521052
Assertions.assertEquals("ADD PRIMARY KEY (`col_1`, `col_2`)", sql);
10531053

1054-
TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1", false);
1055-
sql = MysqlTableOperations.deleteIndexDefinition(null, deleteIndex);
1054+
String tableName = RandomStringUtils.randomAlphabetic(16) + "_op_table";
1055+
String tableComment = "test_comment";
1056+
List<JdbcColumn> columns = new ArrayList<>();
1057+
columns.add(
1058+
JdbcColumn.builder()
1059+
.withName("col_1")
1060+
.withType(INT)
1061+
.withComment("id")
1062+
.withNullable(false)
1063+
.build());
1064+
columns.add(
1065+
JdbcColumn.builder()
1066+
.withName("col_2")
1067+
.withType(INT)
1068+
.withNullable(true)
1069+
.withDefaultValue(Literals.NULL)
1070+
.build());
1071+
Map<String, String> properties = new HashMap<>();
1072+
properties.put(MYSQL_AUTO_INCREMENT_OFFSET_KEY, "10");
1073+
1074+
Index[] indexes = new Index[] {Indexes.unique("uk_2", new String[][] {{"col_1"}})};
1075+
// create table
1076+
TABLE_OPERATIONS.create(
1077+
TEST_DB_NAME.toString(),
1078+
tableName,
1079+
columns.toArray(new JdbcColumn[0]),
1080+
tableComment,
1081+
properties,
1082+
null,
1083+
Distributions.NONE,
1084+
indexes);
1085+
1086+
// load table
1087+
JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName);
1088+
1089+
// If ifExists is set to true then the code should not throw an exception if the index doesn't
1090+
// exist.
1091+
TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1", true);
1092+
sql = MysqlTableOperations.deleteIndexDefinition(load, deleteIndex);
10561093
Assertions.assertEquals("DROP INDEX `uk_1`", sql);
1094+
1095+
// The index existence check should only verify existence when ifExists is false, preventing
1096+
// failures when dropping non-existent indexes.
1097+
TableChange.DeleteIndex deleteIndex2 = new TableChange.DeleteIndex("uk_1", false);
1098+
IllegalArgumentException thrown =
1099+
Assertions.assertThrows(
1100+
IllegalArgumentException.class,
1101+
() -> MysqlTableOperations.deleteIndexDefinition(load, deleteIndex2));
1102+
Assertions.assertEquals("Index does not exist", thrown.getMessage());
1103+
1104+
TableChange.DeleteIndex deleteIndex3 = new TableChange.DeleteIndex("uk_2", false);
1105+
sql = MysqlTableOperations.deleteIndexDefinition(load, deleteIndex3);
1106+
Assertions.assertEquals("DROP INDEX `uk_2`", sql);
10571107
}
10581108
}

‎catalogs/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,11 +361,11 @@ private String updateColumnAutoIncrementDefinition(
361361
@VisibleForTesting
362362
static String deleteIndexDefinition(
363363
JdbcTable lazyLoadTable, TableChange.DeleteIndex deleteIndex) {
364-
if (deleteIndex.isIfExists()) {
365-
if (Arrays.stream(lazyLoadTable.index())
366-
.anyMatch(index -> index.name().equals(deleteIndex.getName()))) {
367-
throw new IllegalArgumentException("Index does not exist");
368-
}
364+
if (!deleteIndex.isIfExists()) {
365+
Preconditions.checkArgument(
366+
Arrays.stream(lazyLoadTable.index())
367+
.anyMatch(index -> index.name().equals(deleteIndex.getName())),
368+
"Index does not exist");
369369
}
370370
return "DROP INDEX " + BACK_QUOTE + deleteIndex.getName() + BACK_QUOTE;
371371
}

‎catalogs/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -990,8 +990,57 @@ public void testOperationIndexDefinition() {
990990
sql = OceanBaseTableOperations.addIndexDefinition(successIndex);
991991
Assertions.assertEquals("ADD PRIMARY KEY (`col_1`, `col_2`)", sql);
992992

993-
TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1", false);
994-
sql = OceanBaseTableOperations.deleteIndexDefinition(null, deleteIndex);
993+
String tableName = RandomStringUtils.randomAlphabetic(16).toLowerCase() + "_oi_table";
994+
String tableComment = "test_comment";
995+
List<JdbcColumn> columns = new ArrayList<>();
996+
columns.add(
997+
JdbcColumn.builder()
998+
.withName("col_1")
999+
.withType(INT)
1000+
.withNullable(false)
1001+
.withComment("set primary key")
1002+
.build());
1003+
columns.add(
1004+
JdbcColumn.builder()
1005+
.withName("col_2")
1006+
.withType(VARCHAR)
1007+
.withComment("test_comment")
1008+
.withNullable(true)
1009+
.build());
1010+
Map<String, String> properties = new HashMap<>();
1011+
1012+
Index[] indexes = new Index[] {Indexes.unique("uk_2", new String[][] {{"col_1"}})};
1013+
// create table
1014+
TABLE_OPERATIONS.create(
1015+
TEST_DB_NAME,
1016+
tableName,
1017+
columns.toArray(new JdbcColumn[0]),
1018+
tableComment,
1019+
properties,
1020+
null,
1021+
Distributions.NONE,
1022+
indexes);
1023+
1024+
// load table
1025+
JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
1026+
1027+
// If ifExists is set to true then the code should not throw an exception if the index doesn't
1028+
// exist.
1029+
TableChange.DeleteIndex deleteIndex = new TableChange.DeleteIndex("uk_1", true);
1030+
sql = OceanBaseTableOperations.deleteIndexDefinition(load, deleteIndex);
9951031
Assertions.assertEquals("DROP INDEX `uk_1`", sql);
1032+
1033+
// The index existence check should only verify existence when ifExists is false, preventing
1034+
// failures when dropping non-existent indexes.
1035+
TableChange.DeleteIndex deleteIndex2 = new TableChange.DeleteIndex("uk_1", false);
1036+
IllegalArgumentException thrown =
1037+
Assertions.assertThrows(
1038+
IllegalArgumentException.class,
1039+
() -> OceanBaseTableOperations.deleteIndexDefinition(load, deleteIndex2));
1040+
Assertions.assertEquals("Index does not exist", thrown.getMessage());
1041+
1042+
TableChange.DeleteIndex deleteIndex3 = new TableChange.DeleteIndex("uk_2", false);
1043+
sql = OceanBaseTableOperations.deleteIndexDefinition(load, deleteIndex3);
1044+
Assertions.assertEquals("DROP INDEX `uk_2`", sql);
9961045
}
9971046
}

0 commit comments

Comments
 (0)