(hudi) branch master updated: [MINOR] Validate that file group reader reads entire files for Bootstrap and MOR queries (#11789)

[yi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

yihua pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git


The following commit(s) were added to refs/heads/master by this push:
     new 7137bc6096d [MINOR] Validate that file group reader reads entire files for Bootstrap and MOR queries (#11789)
7137bc6096d is described below

commit 7137bc6096dd060efa64d94668935ef1ec4808f6
Author: Jon Vexler <jb...@gmail.com>
AuthorDate: Thu Sep 5 01:45:29 2024 -0400

    [MINOR] Validate that file group reader reads entire files for Bootstrap and MOR queries (#11789)
---
 .../common/table/read/HoodieFileGroupReader.java   |  6 +++++
 .../read/HoodieFileGroupReaderSchemaHandler.java   |  4 ++-
 .../table/read/TestHoodieFileGroupReaderBase.java  | 29 ++++++++++++++++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReader.java b/hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReader.java
index e99d076e62f..6467b22aac4 100644
--- a/hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReader.java
+++ b/hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReader.java
@@ -110,6 +110,9 @@ public final class HoodieFileGroupReader<T> implements Closeable {
     readerContext.setLatestCommitTime(latestCommitTime);
     readerContext.setShouldMergeUseRecordPosition(shouldUseRecordPosition);
     readerContext.setHasLogFiles(!this.logFiles.isEmpty());
+    if (readerContext.getHasLogFiles() && start != 0) {
+      throw new IllegalArgumentException("Filegroup reader is doing log file merge but not reading from the start of the base file");
+    }
     readerContext.setHasBootstrapBaseFile(hoodieBaseFileOption.isPresent() && hoodieBaseFileOption.get().getBootstrapBaseFile().isPresent());
     readerContext.setSchemaHandler(readerContext.supportsParquetRowIndex()
         ? new HoodiePositionBasedSchemaHandler<>(readerContext, dataSchema, requestedSchema, internalSchemaOpt, tableConfig)
@@ -175,6 +178,9 @@ public final class HoodieFileGroupReader<T> implements Closeable {
     } else if (!skeletonFileIterator.isPresent()) {
       return dataFileIterator.get().getLeft();
     } else {
+      if (start != 0) {
+        throw new IllegalArgumentException("Filegroup reader is doing bootstrap merge but we are not reading from the start of the base file");
+      }
       return readerContext.mergeBootstrapReaders(skeletonFileIterator.get().getLeft(), skeletonFileIterator.get().getRight(),
           dataFileIterator.get().getLeft(), dataFileIterator.get().getRight());
     }
diff --git a/hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReaderSchemaHandler.java b/hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReaderSchemaHandler.java
index 46d2fdd0c5e..5ded8e622e7 100644
--- a/hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReaderSchemaHandler.java
+++ b/hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReaderSchemaHandler.java
@@ -24,6 +24,7 @@ import org.apache.hudi.common.model.HoodieRecord;
 import org.apache.hudi.common.model.HoodieRecordMerger;
 import org.apache.hudi.common.table.HoodieTableConfig;
 import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.VisibleForTesting;
 import org.apache.hudi.common.util.collection.Pair;
 import org.apache.hudi.internal.schema.InternalSchema;
 import org.apache.hudi.internal.schema.convert.AvroInternalSchemaConverter;
@@ -178,7 +179,8 @@ public class HoodieFileGroupReaderSchemaHandler<T> {
     return getDataAndMetaCols(dataSchema);
   }
 
-  private static Pair<List<Schema.Field>, List<Schema.Field>> getDataAndMetaCols(Schema schema) {
+  @VisibleForTesting
+  static Pair<List<Schema.Field>, List<Schema.Field>> getDataAndMetaCols(Schema schema) {
     Map<Boolean, List<Schema.Field>> fieldsByMeta = schema.getFields().stream()
         //if there are no data fields, then we don't want to think the temp col is a data col
         .filter(f -> !Objects.equals(f.name(), HoodiePositionBasedFileGroupRecordBuffer.ROW_INDEX_TEMPORARY_COLUMN_NAME))
diff --git a/hudi-common/src/test/java/org/apache/hudi/common/table/read/TestHoodieFileGroupReaderBase.java b/hudi-common/src/test/java/org/apache/hudi/common/table/read/TestHoodieFileGroupReaderBase.java
index 6c7b3400427..a53583e63da 100644
--- a/hudi-common/src/test/java/org/apache/hudi/common/table/read/TestHoodieFileGroupReaderBase.java
+++ b/hudi-common/src/test/java/org/apache/hudi/common/table/read/TestHoodieFileGroupReaderBase.java
@@ -40,6 +40,7 @@ import org.apache.hudi.common.testutils.HoodieTestDataGenerator;
 import org.apache.hudi.common.testutils.HoodieTestUtils;
 import org.apache.hudi.common.util.Option;
 import org.apache.hudi.common.util.collection.ExternalSpillableMap;
+import org.apache.hudi.common.util.collection.Pair;
 import org.apache.hudi.keygen.constant.KeyGeneratorOptions;
 import org.apache.hudi.metadata.HoodieTableMetadata;
 import org.apache.hudi.storage.StorageConfiguration;
@@ -285,6 +286,22 @@ public abstract class TestHoodieFileGroupReaderBase<T> {
       props.setProperty(PARTITION_FIELDS.key(), metaClient.getTableConfig().getString(PARTITION_FIELDS));
     }
     assertEquals(containsBaseFile, fileSlice.getBaseFile().isPresent());
+    if (shouldValidatePartialRead(fileSlice, avroSchema)) {
+      assertThrows(IllegalArgumentException.class, () -> new HoodieFileGroupReader<>(
+          getHoodieReaderContext(tablePath, avroSchema, storageConf),
+          metaClient.getStorage(),
+          tablePath,
+          metaClient.getActiveTimeline().lastInstant().get().getTimestamp(),
+          fileSlice,
+          avroSchema,
+          avroSchema,
+          Option.empty(),
+          metaClient,
+          props,
+          1,
+          fileSlice.getTotalFileSize(),
+          false));
+    }
     HoodieFileGroupReader<T> fileGroupReader = new HoodieFileGroupReader<>(
         getHoodieReaderContext(tablePath, avroSchema, storageConf),
         metaClient.getStorage(),
@@ -307,4 +324,16 @@ public abstract class TestHoodieFileGroupReaderBase<T> {
 
     validateRecordsInFileGroup(tablePath, actualRecordList, avroSchema, fileSlice.getFileId());
   }
+
+  private boolean shouldValidatePartialRead(FileSlice fileSlice, Schema requestedSchema) {
+    if (fileSlice.getLogFiles().findAny().isPresent()) {
+      return true;
+    }
+    if (fileSlice.getBaseFile().get().getBootstrapBaseFile().isPresent()) {
+      //TODO: [HUDI-8169] this code path will not hit until we implement bootstrap tests
+      Pair<List<Schema.Field>, List<Schema.Field>> dataAndMetaCols = HoodieFileGroupReaderSchemaHandler.getDataAndMetaCols(requestedSchema);
+      return !dataAndMetaCols.getLeft().isEmpty() && !dataAndMetaCols.getRight().isEmpty();
+    }
+    return false;
+  }
 }