vinishjail97 commented on code in PR #19111:
URL: https://github.com/apache/hudi/pull/19111#discussion_r3500226333
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -236,6 +236,18 @@ public final class HoodieMetadataConfig extends
HoodieConfig {
+ "For the Spark engine, this config defaults to true (enabled),
overriding the base default of false. "
+ "For Flink and Java engines, this remains false by default.");
+ public static final ConfigProperty<Boolean>
ENABLE_METADATA_INDEX_PARTITION_STATS = ConfigProperty
+ .key(METADATA_PREFIX + ".index.partition.stats.enable")
+ .defaultValue(true)
+ .markAdvanced()
+ .sinceVersion("1.0.0")
Review Comment:
Done in 2461b8252d — changed to `.sinceVersion("1.3.0")`. This key was
`@Deprecated` and is being reintroduced now, so 1.3.0 is the accurate version.
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -929,7 +941,11 @@ private Map<String, String>
getExpressionIndexOptions(String configValue) {
}
public boolean isPartitionStatsIndexEnabled() {
- return getBooleanOrDefault(ENABLE_METADATA_INDEX_COLUMN_STATS);
+ // Partition stats are derived from the column stats index, so they can
only be enabled when column
+ // stats is enabled. Within that constraint the partition stats flag is
honoured: it defaults to true
+ // (mirroring column stats) but can be explicitly disabled, e.g. for
tables of externally created files.
+ return getBooleanOrDefault(ENABLE_METADATA_INDEX_COLUMN_STATS)
+ && getBooleanOrDefault(ENABLE_METADATA_INDEX_PARTITION_STATS);
Review Comment:
Good catch — confirmed the gap was real.
`HoodieWriteConfig.isPartitionStatsIndexEnabled()` returned only
`isMetadataColumnStatsIndexEnabled()`, so the build path
(`MetadataPartitionType.PARTITION_STATS.isMetadataPartitionEnabled`, which
honors the new flag) and the delete path (`HoodieTable.isMetadataIndexDisabled`
→ this method) diverged: with column stats on and partition stats off, build
stops writing partition stats but delete wouldn't drop an existing
partition-stats index, leaving it stale.
Fixed in 2461b8252d by delegating to
`getMetadataConfig().isPartitionStatsIndexEnabled()` (which already requires
column stats), so build and delete stay consistent.
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -236,6 +236,18 @@ public final class HoodieMetadataConfig extends
HoodieConfig {
+ "For the Spark engine, this config defaults to true (enabled),
overriding the base default of false. "
+ "For Flink and Java engines, this remains false by default.");
+ public static final ConfigProperty<Boolean>
ENABLE_METADATA_INDEX_PARTITION_STATS = ConfigProperty
Review Comment:
Added in 2461b8252d:
`TestPartitionStatsIndex#testColumnStatsEnabledWithPartitionStatsDisabled`
(parameterized over COW/MOR). It writes a partitioned table with column stats
enabled and `hoodie.metadata.index.partition.stats.enable=false`, then asserts
the MDT contains `COLUMN_STATS` but not `PARTITION_STATS`. Passes locally
(Tests run: 2, Failures: 0, Errors: 0).
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieMetadataConfig.java:
##########
@@ -929,7 +941,11 @@ private Map<String, String>
getExpressionIndexOptions(String configValue) {
}
public boolean isPartitionStatsIndexEnabled() {
- return getBooleanOrDefault(ENABLE_METADATA_INDEX_COLUMN_STATS);
+ // Partition stats are derived from the column stats index, so they can
only be enabled when column
+ // stats is enabled. Within that constraint the partition stats flag is
honoured: it defaults to true
+ // (mirroring column stats) but can be explicitly disabled, e.g. for
tables of externally created files.
+ return getBooleanOrDefault(ENABLE_METADATA_INDEX_COLUMN_STATS)
+ && getBooleanOrDefault(ENABLE_METADATA_INDEX_PARTITION_STATS);
Review Comment:
Confirmed and fixed in 2461b8252d.
`HoodieWriteConfig.isPartitionStatsIndexEnabled()` now delegates to
`getMetadataConfig().isPartitionStatsIndexEnabled()`, so the build and delete
paths agree when partition stats is disabled while column stats stays enabled
(no stale partition-stats index).
--
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]
