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 3704b7a5bf [#12736] fix(doris): validate ADD INDEX fields (#12737)
3704b7a5bf is described below
commit 3704b7a5bfe978421e43d53d5472f6c52bc15073
Author: StormSpirit <[email protected]>
AuthorDate: Mon Aug 31 20:57:27 2026 +0800
[#12736] fix(doris): validate ADD INDEX fields (#12737)
### What changes were proposed in this pull request?
This pull request adds a Doris-specific field-shape validator shared by
CREATE TABLE index generation and ALTER TABLE ADD INDEX. The validator
requires exactly one non-blank top-level field and returns that
validated field for SQL generation.
It also adds focused tests for valid index SQL, malformed field shapes,
consistent CREATE and ALTER validation, the no-JDBC-statement failure
path, and the real Doris 4.x ADD/DROP INDEX lifecycle.
### Why are the changes needed?
The Doris ALTER ADD INDEX path currently reads only
`getFieldNames()[0][0]`. When a request contains multiple fields or a
nested field path, Gravitino silently discards the remaining components
and can create a valid single-column Doris index that does not match the
request.
Rejecting unsupported field shapes before DDL generation prevents this
semantic truncation and makes CREATE and ALTER enforce the same Doris
connector contract. Valid single-field index SQL and existing key-model
behavior remain unchanged.
Fix: #12736
### Does this PR introduce _any_ user-facing change?
Yes. Doris CREATE TABLE and ALTER TABLE ADD INDEX requests containing
multiple fields, nested field paths, empty field shapes, or blank field
names now fail with an explicit `IllegalArgumentException` instead of
being truncated or producing invalid SQL. Valid single-field index
operations are unchanged, and no public API or property key is added or
removed.
### How was this patch tested?
- `./gradlew :catalogs:catalog-jdbc-doris:spotlessCheck`
- `./gradlew rat`
- `./gradlew :catalogs:catalog-jdbc-doris:test -PskipITs`
- `./gradlew :catalogs:catalog-jdbc-doris:test --tests
'org.apache.gravitino.catalog.doris.integration.test.CatalogDoris4xIT.testAddAndDropInvertedIndex'
-PskipDockerTests=false -PdorisMultiVersionTest`
- `./gradlew :catalogs:catalog-jdbc-doris:build -x test`
Signed-off-by: jiangxt2 <[email protected]>
---
.../doris/operation/DorisTableOperations.java | 44 +++++++----
.../doris/integration/test/CatalogDoris4xIT.java | 18 +++++
.../TestDorisTableOperationsSqlGeneration.java | 90 ++++++++++++++++++++++
3 files changed, 136 insertions(+), 16 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 657dc04c12..b002af801b 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
@@ -344,25 +344,18 @@ public class DorisTableOperations extends
JdbcTableOperations {
return;
}
- nonKeyIndexes.forEach(
- index -> {
- if (index.fieldNames().length > 1) {
- throw new IllegalArgumentException(
- "Index '" + index.name() + "' does not support multi fields in
Doris");
- }
- });
-
String indexSql =
nonKeyIndexes.stream()
.map(
index -> {
+ String fieldName =
+ requireSingleTopLevelIndexField(index.name(),
index.fieldNames());
String usingClause = mapIndexTypeToUsingClause(index.type());
if (usingClause.isEmpty()) {
- return String.format(
- "INDEX `%s` (`%s`)", index.name(),
index.fieldNames()[0][0]);
+ return String.format("INDEX `%s` (`%s`)", index.name(),
fieldName);
}
return String.format(
- "INDEX `%s` (`%s`) %s", index.name(),
index.fieldNames()[0][0], usingClause);
+ "INDEX `%s` (`%s`) %s", index.name(), fieldName,
usingClause);
})
.collect(Collectors.joining(",\n"));
@@ -1012,14 +1005,13 @@ public class DorisTableOperations extends
JdbcTableOperations {
throw new UnsupportedOperationException(
"PRIMARY_KEY and UNIQUE_KEY cannot be added via ALTER TABLE ADD
INDEX in Doris");
}
+ String fieldName =
+ requireSingleTopLevelIndexField(addIndex.getName(),
addIndex.getFieldNames());
String usingClause = mapIndexTypeToUsingClause(addIndex.getType());
if (usingClause.isEmpty()) {
- return String.format(
- "ADD INDEX `%s` (`%s`)", addIndex.getName(),
addIndex.getFieldNames()[0][0]);
+ return String.format("ADD INDEX `%s` (`%s`)", addIndex.getName(),
fieldName);
}
- return String.format(
- "ADD INDEX `%s` (`%s`) %s",
- addIndex.getName(), addIndex.getFieldNames()[0][0], usingClause);
+ return String.format("ADD INDEX `%s` (`%s`) %s", addIndex.getName(),
fieldName, usingClause);
}
static String deleteIndexDefinition(
@@ -1082,4 +1074,24 @@ public class DorisTableOperations extends
JdbcTableOperations {
return null;
}
+
+ private static String requireSingleTopLevelIndexField(String indexName,
String[][] fieldNames) {
+ Preconditions.checkArgument(
+ fieldNames != null && fieldNames.length == 1,
+ "Index '%s' supports exactly one top-level field in Doris, but got %s",
+ indexName,
+ fieldNames == null ? "null" : fieldNames.length);
+
+ String[] fieldPath = fieldNames[0];
+ Preconditions.checkArgument(
+ fieldPath != null && fieldPath.length == 1,
+ "Index '%s' supports exactly one top-level field in Doris, but got
path %s",
+ indexName,
+ Arrays.toString(fieldPath));
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(fieldPath[0]),
+ "Index '%s' requires a non-blank top-level field in Doris",
+ indexName);
+ return fieldPath[0];
+ }
}
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java
index 39af05faf7..d438a0e8d8 100644
---
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/integration/test/CatalogDoris4xIT.java
@@ -26,6 +26,7 @@ import static
org.apache.gravitino.integration.test.util.ITUtils.assertPartition
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import com.google.common.collect.Maps;
@@ -197,6 +198,23 @@ public class CatalogDoris4xIT extends BaseIT {
null,
null);
+ IllegalArgumentException exception =
+ assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ tc.alterTable(
+ tid,
+ TableChange.addIndex(
+ Index.IndexType.INVERTED,
+ "idx_invalid",
+ new String[][] {{colName1}, {colName2}})));
+ assertTrue(
+ exception
+ .getMessage()
+ .contains(
+ "Index 'idx_invalid' supports exactly one top-level field in
Doris, but got 2"));
+ assertEquals(0, tc.loadTable(tid).index().length);
+
// Add INVERTED index
tc.alterTable(
tid,
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
index ba7b665c01..af1bc0dc47 100644
---
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
@@ -447,6 +447,84 @@ public class TestDorisTableOperationsSqlGeneration {
Assertions.assertEquals("ADD INDEX `idx_vec` (`embedding`) USING ANN",
sql);
}
+ @Test
+ public void testAddIndexDefinitionRejectsUnsupportedFieldShapes() {
+ assertInvalidAddIndex(
+ new String[][] {{"col1"}, {"col2"}},
+ "Index 'idx_name' supports exactly one top-level field in Doris, but
got 2");
+ assertInvalidAddIndex(
+ new String[][] {{"payload", "nested"}},
+ "Index 'idx_name' supports exactly one top-level field in Doris, but
got path "
+ + "[payload, nested]");
+ assertInvalidAddIndex(
+ new String[0][],
+ "Index 'idx_name' supports exactly one top-level field in Doris, but
got 0");
+ assertInvalidAddIndex(
+ new String[][] {new String[0]},
+ "Index 'idx_name' supports exactly one top-level field in Doris, but
got path []");
+ assertInvalidAddIndex(
+ new String[][] {null},
+ "Index 'idx_name' supports exactly one top-level field in Doris, but
got path null");
+ assertInvalidAddIndex(
+ new String[][] {{" "}}, "Index 'idx_name' requires a non-blank
top-level field in Doris");
+ assertInvalidAddIndex(
+ null, "Index 'idx_name' supports exactly one top-level field in Doris,
but got null");
+ }
+
+ @Test
+ public void testCreateAndAlterIndexUseSameFieldShapeValidation() {
+ TestableDorisTableOperations ops = new TestableDorisTableOperations();
+ TestableDorisTableOperations mockOps = Mockito.spy(ops);
+ Mockito.doAnswer(a -> a.getArgument(0))
+ .when(mockOps)
+ .appendNecessaryProperties(Mockito.anyMap());
+ JdbcColumn idColumn =
+ JdbcColumn.builder()
+ .withName("id")
+ .withType(Types.IntegerType.get())
+ .withNullable(false)
+ .build();
+ Distribution distribution = Distributions.hash(1,
NamedReference.field("id"));
+ String[][] fields = {{"id"}, {"other"}};
+ Index[] indexes = {Indexes.of(Index.IndexType.INVERTED, "idx_name",
fields)};
+
+ IllegalArgumentException createException =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ mockOps.createTableSqlWithIndexes(
+ "test_table", new JdbcColumn[] {idColumn}, distribution,
indexes));
+ IllegalArgumentException alterException =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ mockOps.alterTableSql(
+ "test_table",
+ TableChange.addIndex(Index.IndexType.INVERTED, "idx_name",
fields)));
+
+ Assertions.assertEquals(createException.getMessage(),
alterException.getMessage());
+ }
+
+ @Test
+ public void testInvalidAddIndexDoesNotExecuteJdbcStatement() throws
Exception {
+ TestableDorisTableOperations ops = new TestableDorisTableOperations();
+ DataSource dataSource = Mockito.mock(DataSource.class);
+ Connection connection = Mockito.mock(Connection.class);
+ Mockito.when(dataSource.getConnection()).thenReturn(connection);
+ ops.setDataSource(dataSource);
+
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.alterTable(
+ "database",
+ "test_table",
+ TableChange.addIndex(
+ Index.IndexType.INVERTED, "idx_name", new String[][]
{{"col1"}, {"col2"}})));
+
+ Mockito.verify(connection, Mockito.never()).createStatement();
+ }
+
@Test
public void testAddPrimaryKeyIndexDefinitionThrows() {
// PRIMARY_KEY cannot be added via ALTER TABLE ADD INDEX in Doris
@@ -546,6 +624,18 @@ public class TestDorisTableOperationsSqlGeneration {
exception.getMessage());
}
+ private static void assertInvalidAddIndex(String[][] fields, String
expectedMessage) {
+ TableChange.AddIndex addIndex =
+ (TableChange.AddIndex) TableChange.addIndex(Index.IndexType.INVERTED,
"idx_name", fields);
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> DorisTableOperations.addIndexDefinition(addIndex));
+
+ Assertions.assertEquals(expectedMessage, exception.getMessage());
+ }
+
private static DataSource mockBackendDataSource(int aliveBackendCount)
throws Exception {
DataSource dataSource = Mockito.mock(DataSource.class);
Connection connection = Mockito.mock(Connection.class);
