the-other-tim-brown commented on code in PR #13602:
URL: https://github.com/apache/hudi/pull/13602#discussion_r2231948762


##########
hudi-common/src/test/java/org/apache/hudi/common/table/read/TestFileGroupReaderSchemaHandler.java:
##########
@@ -111,4 +123,127 @@ FileGroupReaderSchemaHandler 
createSchemaHandler(HoodieReaderContext<String> rea
     return new FileGroupReaderSchemaHandler(readerContext, dataSchema, 
requestedSchema,
         Option.empty(), hoodieTableConfig, new TypedProperties());
   }
+
+  @ParameterizedTest
+  @CsvSource({
+      "true, true, true, EVENT_TIME_ORDERING, false, EIGHT, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5",
+      "true, false, false, EVENT_TIME_ORDERING, false, EIGHT, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5",
+      "false, true, false, EVENT_TIME_ORDERING, false, EIGHT, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5",
+      "false, false, true, EVENT_TIME_ORDERING, false, EIGHT, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5",
+      "true, true, true, COMMIT_TIME_ORDERING, false, EIGHT, 
ce9acb64-bde0-424c-9b91-f6ebba25356d",
+      "true, false, false, COMMIT_TIME_ORDERING, false, EIGHT, 
ce9acb64-bde0-424c-9b91-f6ebba25356d",
+      "false, true, false, COMMIT_TIME_ORDERING, false, EIGHT, 
ce9acb64-bde0-424c-9b91-f6ebba25356d",
+      "false, false, true, COMMIT_TIME_ORDERING, false, EIGHT, 
ce9acb64-bde0-424c-9b91-f6ebba25356d",
+      "true, true, true, CUSTOM, false, EIGHT, 
00000000-0000-0000-0000-000000000000",
+      "true, false, false, CUSTOM, false, EIGHT, 
00000000-0000-0000-0000-000000000000",
+      "false, true, false, CUSTOM, false, EIGHT, 
00000000-0000-0000-0000-000000000000",
+      "false, false, true, CUSTOM, false, EIGHT, 
00000000-0000-0000-0000-000000000000",
+      "true, true, true, , false, EIGHT, 00000000-0000-0000-0000-000000000000",
+      "true, false, false, , false, EIGHT, 
00000000-0000-0000-0000-000000000000",
+      "false, true, false, , false, EIGHT, 
00000000-0000-0000-0000-000000000000",
+      "false, false, true, , false, EIGHT, 
00000000-0000-0000-0000-000000000000",
+      "true, true, true, EVENT_TIME_ORDERING, false, SIX, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5",
+      "true, false, false, EVENT_TIME_ORDERING, false, SIX, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5",
+      "false, true, false, EVENT_TIME_ORDERING, false, SIX, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5",
+      "false, false, true, EVENT_TIME_ORDERING, false, SIX, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5",
+      "true, true, true, COMMIT_TIME_ORDERING, false, SIX, 
ce9acb64-bde0-424c-9b91-f6ebba25356d",
+      "true, false, false, COMMIT_TIME_ORDERING, false, SIX, 
ce9acb64-bde0-424c-9b91-f6ebba25356d",
+      "false, true, false, COMMIT_TIME_ORDERING, false, SIX, 
ce9acb64-bde0-424c-9b91-f6ebba25356d",
+      "false, false, true, COMMIT_TIME_ORDERING, false, SIX, 
ce9acb64-bde0-424c-9b91-f6ebba25356d",
+      "true, true, true, CUSTOM, false, SIX, 
00000000-0000-0000-0000-000000000000",
+      "true, false, false, CUSTOM, false, SIX, 
00000000-0000-0000-0000-000000000000",
+      "false, true, false, CUSTOM, false, SIX, 
00000000-0000-0000-0000-000000000000",
+      "false, false, true, CUSTOM, false, SIX, 
00000000-0000-0000-0000-000000000000",
+      "true, true, true, , false, SIX, 00000000-0000-0000-0000-000000000000",
+      "true, false, false, , false, SIX, 00000000-0000-0000-0000-000000000000",
+      "false, true, false, , false, SIX, 00000000-0000-0000-0000-000000000000",
+      "false, false, true, , false, SIX, 00000000-0000-0000-0000-000000000000",
+      "true, true, true, COMMIT_TIME_ORDERING, true, SIX, 
eeb8d96f-b1e4-49fd-bbf8-28ac514178e5", /// with table version 6, commit time 
based merge mode can have event time based merge strategy id.
+  })
+  public void testSchemaForMandatoryFields(boolean setPrecombine,
+                                           boolean addHoodieIsDeleted,
+                                           boolean addCustomDeleteMarker,
+                                           RecordMergeMode mergeMode,
+                                           boolean isProjectionCompatible,
+                                           HoodieTableVersion tableVersion,
+                                           String mergeStrategyId) {
+    HoodieReaderContext readerContext = mock(HoodieReaderContext.class);
+    when(readerContext.getInstantRange()).thenReturn(Option.empty());
+    when(readerContext.getHasBootstrapBaseFile()).thenReturn(false);
+    when(readerContext.getHasLogFiles()).thenReturn(true);
+    HoodieRecordMerger recordMerger = mock(HoodieRecordMerger.class);
+    when(readerContext.getRecordMerger()).thenReturn(Option.of(recordMerger));
+    
when(recordMerger.isProjectionCompatible()).thenReturn(isProjectionCompatible);
+
+    String preCombineField = "ts";
+    String customDeleteKey = "colC";
+    String customDeleteValue = "D";
+    List<String> dataSchemaFields = new ArrayList<>();
+    dataSchemaFields.addAll(Arrays.asList(
+        HoodieRecord.RECORD_KEY_METADATA_FIELD, 
HoodieRecord.PARTITION_PATH_METADATA_FIELD, preCombineField,
+        "colA", "colB", "colC", "colD"));
+    if (addHoodieIsDeleted) {
+      dataSchemaFields.add(HoodieRecord.HOODIE_IS_DELETED_FIELD);
+    }
+
+    Schema dataSchema = getSchema(dataSchemaFields);
+    Schema requestedSchema = 
getSchema(Arrays.asList(HoodieRecord.RECORD_KEY_METADATA_FIELD, 
HoodieRecord.PARTITION_PATH_METADATA_FIELD));
+
+    HoodieTableConfig tableConfig = mock(HoodieTableConfig.class);
+    when(tableConfig.getRecordMergeMode()).thenReturn(mergeMode);
+    when(tableConfig.populateMetaFields()).thenReturn(true);
+    when(tableConfig.getPreCombineField()).thenReturn(setPrecombine ? 
preCombineField : StringUtils.EMPTY_STRING);
+    when(tableConfig.getTableVersion()).thenReturn(tableVersion);
+    if (tableConfig.getTableVersion() == HoodieTableVersion.SIX) {
+      if (mergeMode == RecordMergeMode.EVENT_TIME_ORDERING) {
+        
when(tableConfig.getPayloadClass()).thenReturn(DefaultHoodieRecordPayload.class.getName());
+      } else if (mergeMode == RecordMergeMode.COMMIT_TIME_ORDERING) {
+        
when(tableConfig.getPayloadClass()).thenReturn(OverwriteWithLatestAvroPayload.class.getName());
+      } else {
+        
when(tableConfig.getPayloadClass()).thenReturn(OverwriteNonDefaultsWithLatestAvroPayload.class.getName());
+      }
+    }
+    if (mergeMode != null) {
+      when(tableConfig.getRecordMergeStrategyId()).thenReturn(mergeStrategyId);
+    }
+
+    TypedProperties props = new TypedProperties();
+    if (addCustomDeleteMarker) {
+      props.setProperty(DELETE_KEY, customDeleteKey);
+      props.setProperty(DELETE_MARKER, customDeleteValue);
+    }
+
+    List<String> expectedFields = new ArrayList();
+    expectedFields.add(HoodieRecord.RECORD_KEY_METADATA_FIELD);
+    expectedFields.add(HoodieRecord.PARTITION_PATH_METADATA_FIELD);
+    if (addCustomDeleteMarker) {
+      expectedFields.add(customDeleteKey);
+    }
+    if (setPrecombine && mergeMode != RecordMergeMode.COMMIT_TIME_ORDERING) { 
// commit time ordering does not project ordering field.
+      expectedFields.add(preCombineField);
+    }
+    if (addHoodieIsDeleted) {
+      expectedFields.add(HoodieRecord.HOODIE_IS_DELETED_FIELD);
+    }
+    Schema expectedSchema = ((mergeMode == RecordMergeMode.CUSTOM) && 
!isProjectionCompatible) ? dataSchema : getSchema(expectedFields);
+    when(recordMerger.getMandatoryFieldsForMerging(dataSchema, tableConfig, 
props)).thenReturn(expectedFields.toArray(new String[0]));
+
+    FileGroupReaderSchemaHandler fileGroupReaderSchemaHandler = new 
FileGroupReaderSchemaHandler(readerContext,
+        dataSchema, requestedSchema, Option.empty(), tableConfig, props);
+    Schema actualSchema = 
fileGroupReaderSchemaHandler.generateRequiredSchema();
+    assertEquals(expectedSchema, actualSchema);
+    assertEquals(addHoodieIsDeleted, 
fileGroupReaderSchemaHandler.hasBuiltInDelete());
+    assertEquals(addCustomDeleteMarker
+            ? Option.of(Pair.of(customDeleteKey, customDeleteValue)) : 
Option.empty(),
+        fileGroupReaderSchemaHandler.getCustomDeleteMarkerKeyValue());
+  }
+
+  private Schema getSchema(List<String> fields) {

Review Comment:
   Moved it to SchemaTestUtils



##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReader.java:
##########
@@ -79,20 +74,15 @@ public final class HoodieFileGroupReader<T> implements 
Closeable {
   private final Option<String> orderingFieldName;
   private final HoodieStorage storage;
   private final TypedProperties props;
+  private final ReaderParameters readerParameters;
+  private final FileGroupRecordBufferLoader<T> recordBufferInitializer;

Review Comment:
   Updated



-- 
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]

Reply via email to