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 ddd53902a5 [Cherry-pick to branch-1.3] [MINOR] fix(postgresql): drop
constraint-backed indexes correctly (#13922) (#13955)
ddd53902a5 is described below
commit ddd53902a5f76c1898c68bab1e0a424d577002f6
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Oct 20 08:19:27 2026 +0800
[Cherry-pick to branch-1.3] [MINOR] fix(postgresql): drop constraint-backed
indexes correctly (#13922) (#13955)
**Cherry-pick Information:**
- Original commit: 8d1b4c14bec9f2cd7ac9d08071c16cb654ca2e8c
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Qi Yu <[email protected]>
---
.../operation/PostgreSqlTableOperations.java | 38 +++++-----------
.../integration/test/CatalogPostgreSqlIT.java | 51 ++++++++++++++++++++++
.../operation/TestPostgreSqlTableOperations.java | 8 +---
...TestPostgreSqlTableOperationsSqlGeneration.java | 36 +++++++++++++++
4 files changed, 100 insertions(+), 33 deletions(-)
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
index 9ce61a0281..75aa318048 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
@@ -414,33 +414,17 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
@VisibleForTesting
static String deleteIndexDefinition(String tableName,
TableChange.DeleteIndex deleteIndex) {
- StringBuilder sqlBuilder = new StringBuilder();
- sqlBuilder
- .append("ALTER TABLE ")
- .append(PG_QUOTE)
- .append(tableName)
- .append(PG_QUOTE)
- .append(" DROP CONSTRAINT ")
- .append(PG_QUOTE)
- .append(deleteIndex.getName())
- .append(PG_QUOTE)
- .append(";\n");
- if (deleteIndex.isIfExists()) {
- sqlBuilder
- .append("DROP INDEX IF EXISTS ")
- .append(PG_QUOTE)
- .append(deleteIndex.getName())
- .append(PG_QUOTE)
- .append(";");
- } else {
- sqlBuilder
- .append("DROP INDEX ")
- .append(PG_QUOTE)
- .append(deleteIndex.getName())
- .append(PG_QUOTE)
- .append(";");
- }
- return sqlBuilder.toString();
+ // Gravitino creates primary and unique indexes as constraints. Dropping
the constraint
+ // also removes its backing index, so a second DROP INDEX would fail after
a successful drop.
+ return String.format(
+ "ALTER TABLE %s%s%s DROP CONSTRAINT %s%s%s%s;",
+ PG_QUOTE,
+ tableName,
+ PG_QUOTE,
+ deleteIndex.isIfExists() ? "IF EXISTS " : "",
+ PG_QUOTE,
+ deleteIndex.getName(),
+ PG_QUOTE);
}
@VisibleForTesting
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
index 9441493386..ec331a1ae8 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
@@ -48,6 +48,7 @@ import org.apache.gravitino.auth.AuthConstants;
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
import
org.apache.gravitino.catalog.postgresql.integration.test.service.PostgreSqlService;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.dto.requests.TableUpdateRequest;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.SchemaAlreadyExistsException;
@@ -84,6 +85,9 @@ import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
@Tag("gravitino-docker-test")
@TestInstance(Lifecycle.PER_CLASS)
@@ -1650,6 +1654,53 @@ public class CatalogPostgreSqlIT extends BaseIT {
Types.ListType.of(Types.IntegerType.get(), true),
loadedTable.columns()[2].dataType());
}
+ @ParameterizedTest
+ @NullSource
+ @ValueSource(booleans = {false, true})
+ void testDeleteConstraintBackedIndexes(Boolean ifExists) {
+ NameIdentifier ident =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName("test_delete_indexes"));
+ TableCatalog tableCatalog = catalog.asTableCatalog();
+ tableCatalog.createTable(
+ ident,
+ new Column[] {
+ Column.of("id", Types.IntegerType.get(), null, false, false, null),
+ Column.of("code", Types.IntegerType.get(), null, false, false, null)
+ },
+ null,
+ createProperties(),
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ new SortOrder[0],
+ new Index[] {
+ Indexes.primary("delete_pk", new String[][] {{"id"}}),
+ Indexes.unique("delete_uk", new String[][] {{"code"}})
+ });
+
+ TableUpdateRequest.DeleteTableIndexRequest primary =
+ new TableUpdateRequest.DeleteTableIndexRequest("delete_pk", ifExists);
+ TableUpdateRequest.DeleteTableIndexRequest unique =
+ new TableUpdateRequest.DeleteTableIndexRequest("delete_uk", ifExists);
+ Table updated = tableCatalog.alterTable(ident, primary.tableChange());
+ Assertions.assertEquals(1, updated.index().length);
+ Assertions.assertEquals("delete_uk", updated.index()[0].name());
+
+ updated = tableCatalog.alterTable(ident, unique.tableChange());
+ Assertions.assertEquals(0, updated.index().length);
+ Assertions.assertEquals(0, tableCatalog.loadTable(ident).index().length);
+
+ // IF EXISTS applies to the constraint, including after both indexes have
been removed.
+ tableCatalog.alterTable(
+ ident,
+ TableChange.deleteIndex("delete_pk", true),
+ TableChange.deleteIndex("delete_uk", true));
+ RuntimeException missing =
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () -> tableCatalog.alterTable(ident,
TableChange.deleteIndex("delete_pk", false)));
+ Assertions.assertTrue(missing.getMessage().contains("delete_pk"),
missing.getMessage());
+ }
+
@Test
void testOperationTableIndex() {
String tableName = GravitinoITUtils.genRandomName("test_add_index");
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
index 9fb4aafa49..cef450d4dc 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
@@ -723,15 +723,11 @@ public class TestPostgreSqlTableOperations extends
TestPostgreSql {
// Test delete index definition.
TableChange.DeleteIndex deleteIndex = new
TableChange.DeleteIndex("test_pk", false);
result = PostgreSqlTableOperations.deleteIndexDefinition(tableName,
deleteIndex);
- Assertions.assertEquals(
- "ALTER TABLE \"t1\" DROP CONSTRAINT \"test_pk\";\n" + "DROP INDEX
\"test_pk\";", result);
+ Assertions.assertEquals("ALTER TABLE \"t1\" DROP CONSTRAINT \"test_pk\";",
result);
deleteIndex = new TableChange.DeleteIndex("test_2_pk", true);
result = PostgreSqlTableOperations.deleteIndexDefinition(tableName,
deleteIndex);
- Assertions.assertEquals(
- "ALTER TABLE \"t1\" DROP CONSTRAINT \"test_2_pk\";\n"
- + "DROP INDEX IF EXISTS \"test_2_pk\";",
- result);
+ Assertions.assertEquals("ALTER TABLE \"t1\" DROP CONSTRAINT IF EXISTS
\"test_2_pk\";", result);
}
@Test
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
index 2862f98412..38f439ac20 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperationsSqlGeneration.java
@@ -24,6 +24,7 @@ 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.postgresql.converter.PostgreSqlTypeConverter;
+import org.apache.gravitino.dto.requests.TableUpdateRequest;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.literals.Literals;
@@ -32,6 +33,8 @@ 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.ValueSource;
public class TestPostgreSqlTableOperationsSqlGeneration {
@@ -78,6 +81,39 @@ public class TestPostgreSqlTableOperationsSqlGeneration {
}
}
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ void testDeleteIndexesDropsOnlyConstraints(boolean ifExists) {
+ TestablePostgreSqlTableOperations ops = new
TestablePostgreSqlTableOperations();
+
+ String sql =
+ ops.alterTableSql(
+ "test_table",
+ TableChange.deleteIndex("primary_key", ifExists),
+ TableChange.deleteIndex("unique_key", ifExists));
+
+ String optional = ifExists ? "IF EXISTS " : "";
+ Assertions.assertEquals(
+ "ALTER TABLE \"test_table\" DROP CONSTRAINT "
+ + optional
+ + "\"primary_key\";\n"
+ + "ALTER TABLE \"test_table\" DROP CONSTRAINT "
+ + optional
+ + "\"unique_key\";",
+ sql);
+ }
+
+ @Test
+ void testDeleteIndexWithoutIfExistsUsesStrictConstraintDrop() {
+ TableUpdateRequest request =
+ new TableUpdateRequest.DeleteTableIndexRequest("primary_key", null);
+ TestablePostgreSqlTableOperations ops = new
TestablePostgreSqlTableOperations();
+
+ Assertions.assertEquals(
+ "ALTER TABLE \"test_table\" DROP CONSTRAINT \"primary_key\";",
+ ops.alterTableSql("test_table", request.tableChange()));
+ }
+
@Test
public void testCreateTableWithEmptyStringDefaultValue() {
TestablePostgreSqlTableOperations ops = new
TestablePostgreSqlTableOperations();
