rangareddy commented on issue #17337:
URL: https://github.com/apache/hudi/issues/17337#issuecomment-5351234205
This issue was reviewed as part of the JIRA-migrated backlog triage
(HUDI-8705).
**Findings: confirmed, unchanged on `master`.**
Your premise is exactly right - the prerequisite is enforced on create but
not on drop.
**Create side** (enforced),
`hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:711-720`:
```java
// Check if record index is enabled for secondary index
...
"Cannot create secondary index '%s': Record index is required for secondary
indexes but is not enabled. "
```
with coverage in `TestHoodieIndexUtils` at `:373-399` ("Should throw
`HoodieMetadataIndexException` as record index is a prerequisite for secondary
index").
**Drop side** (not enforced),
`hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/index/HoodieSparkIndexClient.java:193`:
```java
public void drop(HoodieTableMetaClient metaClient, String indexName, boolean
ignoreIfNotExists) {
log.info("Dropping index {}", indexName);
Option<HoodieIndexDefinition> indexDefinitionOpt =
metaClient.getIndexMetadata()
.map(HoodieIndexMetadata::getIndexDefinitions)
.map(definition -> definition.get(indexName));
try (SparkRDDWriteClient writeClient = getWriteClient(metaClient,
indexDefinitionOpt, Option.empty(), Collections.emptyMap())) {
writeClient.dropIndex(Collections.singletonList(indexName));
}
}
```
It resolves the single named definition and drops it. There is no dependency
walk. `DropIndexCommand` (`IndexCommands.scala:119-141`) behaves the same way.
So dropping `record_index` leaves the secondary index in place and the next
upsert hits the create-time-style error, which matches what you described.
One design question worth settling in the ticket before coding: silently
cascading a drop is a surprising outcome for a user who only asked to drop the
RLI. The alternatives are to refuse the RLI drop while dependent secondary
indexes exist (and tell the user which ones), or to cascade with an explicit
warning. Refusing is arguably safer, but it changes the ergonomics of `DROP
INDEX record_index`. Either way, the check belongs in the shared drop path
rather than in the Spark SQL command, so the datasource and CLI routes get it
too.
Keeping this open.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
