hudi-agent commented on code in PR #20357:
URL: https://github.com/apache/hudi/pull/20357#discussion_r4290874141
##########
hudi-spark-datasource/hudi-spark/src/test/java/org/apache/hudi/functional/TestHoodieFileSystemViews.java:
##########
@@ -85,33 +85,40 @@ public class TestHoodieFileSystemViews extends
HoodieClientTestBase {
@Getter(AccessLevel.PROTECTED)
private HoodieTableType tableType = HoodieTableType.COPY_ON_WRITE;
+ /**
+ * Every table is written with the metadata table enabled and compared
against both the in-memory and the spillable view
+ * (see {@link #viewStorageTypesToValidate}). The writer uses the spillable
view for every older supported table version
+ * and both view storage types for the current table version.
+ */
public static List<Arguments> tableTypeMetadataFSVTypeArgs() {
List<Arguments> testCases = new ArrayList<>();
for (HoodieTableType tableType : HoodieTableType.values()) {
- for (boolean enableMdt : Arrays.asList(true, false)) {
- for (FileSystemViewStorageType viewStorageType :
Arrays.asList(FileSystemViewStorageType.MEMORY,
FileSystemViewStorageType.SPILLABLE_DISK)) {
- if (!enableMdt && viewStorageType ==
FileSystemViewStorageType.MEMORY) {
- // This is the baseline case, no need to test here.
- continue;
- }
- for (int writerVersion : Arrays.asList(6, 8)) {
- testCases.add(Arguments.of(tableType, enableMdt, viewStorageType,
writerVersion));
- }
- }
+ for (HoodieTableVersion olderTableVersion :
Arrays.asList(HoodieTableVersion.SIX, HoodieTableVersion.EIGHT,
HoodieTableVersion.NINE)) {
+ testCases.add(Arguments.of(tableType, true,
FileSystemViewStorageType.SPILLABLE_DISK, olderTableVersion.versionCode()));
+ }
+ for (FileSystemViewStorageType writerViewStorageType :
Arrays.asList(FileSystemViewStorageType.MEMORY,
FileSystemViewStorageType.SPILLABLE_DISK)) {
+ testCases.add(Arguments.of(tableType, true, writerViewStorageType,
HoodieTableVersion.current().versionCode()));
}
}
return testCases;
}
+ private static List<FileSystemViewStorageType>
viewStorageTypesToValidate(boolean enableMdt, FileSystemViewStorageType
writerViewStorageType) {
Review Comment:
🤖 nit: could you drop the `enableMdt` branch here, or the `enableMdt`
argument altogether? Every test case now passes `true`, so the
`singletonList(writerViewStorageType)` path is never run.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
