yihua commented on code in PR #12888:
URL: https://github.com/apache/hudi/pull/12888#discussion_r1981947899
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieReaderConfig.java:
##########
@@ -89,4 +95,18 @@ public class HoodieReaderConfig extends HoodieConfig {
"hoodie.write.record.merge.custom.implementation.classes";
public static final String
RECORD_MERGE_IMPL_CLASSES_DEPRECATED_WRITE_CONFIG_KEY =
"hoodie.datasource.write.record.merger.impls";
+
+ public static boolean isFileGroupReaderEnabled(HoodieTableVersion
tableVersion, HoodieConfig config) {
+ return tableVersion.greaterThanOrEquals(HoodieTableVersion.EIGHT) &&
config.getBooleanOrDefault(HoodieReaderConfig.FILE_GROUP_READER_ENABLED);
+ }
+
+ public static boolean isFileGroupReaderEnabled(HoodieTableVersion
tableVersion, Map<String, String> parameters) {
+ return tableVersion.greaterThanOrEquals(HoodieTableVersion.EIGHT)
+ &&
Boolean.parseBoolean(parameters.getOrDefault(HoodieReaderConfig.FILE_GROUP_READER_ENABLED.key(),
HoodieReaderConfig.FILE_GROUP_READER_ENABLED.defaultValue().toString()));
+ }
+
+ public static boolean isFileGroupReaderEnabled(HoodieTableVersion
tableVersion, Configuration conf) {
+ return tableVersion.greaterThanOrEquals(HoodieTableVersion.EIGHT)
+ && conf.getBoolean(HoodieReaderConfig.FILE_GROUP_READER_ENABLED.key(),
HoodieReaderConfig.FILE_GROUP_READER_ENABLED.defaultValue());
+ }
Review Comment:
Let's remove all these new config logic and fix forward.
In fact, basic functionality of reading file groups in table version 6 is
validated in `TestMergeModeCommitTimeOrdering`,
`TestMergeModeEventTimeOrdering`. So we should dig deeper and understand the
additional issue discovered if there is any.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordScanner.java:
##########
@@ -479,7 +481,7 @@ &&
compareTimestamps(logBlock.getLogBlockHeader().get(INSTANT_TIME), GREATER_THA
continue;
}
if (logBlock.getBlockType() != COMMAND_BLOCK) {
- if (this.tableVersion.lesserThan(HoodieTableVersion.EIGHT)) {
+ if (this.tableVersion.lesserThan(HoodieTableVersion.EIGHT) &&
!allowInflightInstants) {
Review Comment:
Could you fix `HoodieFileGroupReader`, `BaseHoodieLogRecordReader`, and
`HoodieBaseFileGroupRecordBuffer` on the same logic? `*LogRecordScanner`
classes are going to be deprecated.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -632,6 +637,7 @@ static boolean validateConfigVersion(ConfigProperty<?>
configProperty, HoodieTab
// validate that the table version is greater than or equal to the config
version
HoodieTableVersion firstVersion =
HoodieTableVersion.fromReleaseVersion(configProperty.getSinceVersion().get());
boolean valid = tableVersion.greaterThan(firstVersion) ||
tableVersion.equals(firstVersion);
+ valid = valid ||
CONFIGS_REQUIRED_FOR_OLDER_VERSIONED_TABLES.contains(configProperty.key());
Review Comment:
What is the rationale here? It looks like we need to validate this in
another place instead of here.
--
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]
