nsivabalan commented on code in PR #13533:
URL: https://github.com/apache/hudi/pull/13533#discussion_r2217135841
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestMORDataSource.scala:
##########
@@ -321,7 +342,7 @@ class TestMORDataSource extends HoodieSparkClientTestBase
with SparkDatasetMixin
// incremental query from commit2Time
// validate incremental queries only for table version 8
// 1.0 reader (table version 8) supports incremental query reads using
completion time
- if (tableVersion.equals("EIGHT")) {
+ if (tableVersion.equals("CURRENT") || tableVersion.equals("EIGHT")) {
Review Comment:
same comment as above
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestSevenToEightUpgrade.scala:
##########
@@ -75,7 +75,7 @@ class TestSevenToEightUpgrade extends
RecordLevelIndexTestBase {
metaClient = getLatestMetaClient(true)
// assert table version is eight and the partition fields in table config
has partition type
- assertEquals(HoodieTableVersion.EIGHT,
metaClient.getTableConfig.getTableVersion)
+ assertEquals(HoodieTableVersion.current(),
metaClient.getTableConfig.getTableVersion)
Review Comment:
fix comment in L 77 (eight -> nine)
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/utils/TestCommonClientUtils.java:
##########
@@ -95,14 +95,13 @@ private static Stream<Arguments>
provideValidTableVersionWriteVersionPairs() {
generateSameVersionCases()
),
Stream.of(
- // Rule 3: special case - upgrade scenario (table > 6, table < 9,
writer = 6)
+ // Rule 3: downgrade scenarios
Arguments.of(HoodieTableVersion.SEVEN, HoodieTableVersion.SIX,
true),
Arguments.of(HoodieTableVersion.EIGHT, HoodieTableVersion.SIX,
true),
-
- // Rule 4: otherwise disallowed - table > writer (except special
case above)
- Arguments.of(HoodieTableVersion.NINE, HoodieTableVersion.SIX,
false),
- Arguments.of(HoodieTableVersion.NINE, HoodieTableVersion.EIGHT,
false),
- Arguments.of(HoodieTableVersion.EIGHT, HoodieTableVersion.SEVEN,
false)
+ Arguments.of(HoodieTableVersion.NINE, HoodieTableVersion.SIX,
true),
Review Comment:
where are the disallowed combinations.
for eg, 9 -> 5, 8 -> 5, 6 -> 5
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestMORDataSource.scala:
##########
@@ -207,7 +228,7 @@ class TestMORDataSource extends HoodieSparkClientTestBase
with SparkDatasetMixin
.mode(SaveMode.Append)
.save(basePath)
HoodieTestUtils.validateTableConfig(storage, basePath, expectedConfigs,
nonExistentConfigs)
- val commit2CompletionTime = if (tableVersion.equals("EIGHT")) {
+ val commit2CompletionTime = if (tableVersion.equals("CURRENT") ||
tableVersion.equals("EIGHT")) {
Review Comment:
same here
##########
hudi-utilities/src/test/java/org/apache/hudi/utilities/deltastreamer/TestHoodieDeltaStreamer.java:
##########
@@ -670,15 +670,15 @@ public void testSchemaEvolution(String tableType, boolean
useUserProvidedSchema,
private static Stream<Arguments> continuousModeArgs() {
return Stream.of(
- Arguments.of("AVRO", "EIGHT"),
- Arguments.of("SPARK", "EIGHT"),
+ Arguments.of("AVRO", "CURRENT"),
+ Arguments.of("SPARK", "CURRENT"),
Review Comment:
can we do just 1 combo for EIGHT (AVRO, EIGHT)
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/others/TestMergeModeCommitTimeOrdering.scala:
##########
@@ -276,7 +279,8 @@ class TestMergeModeCommitTimeOrdering extends
HoodieSparkSqlTestBase {
storage, tmp.getCanonicalPath, expectedMergeConfigs,
nonExistentConfigs)
// TODO(HUDI-8840): enable MERGE INTO with deletes
- val shouldTestMergeIntoDelete = setRecordMergeConfigs &&
tableVersion.toInt == 8
+ val shouldTestMergeIntoDelete = setRecordMergeConfigs &&
+ tableVersion.toInt == HoodieTableVersion.current().versionCode()
Review Comment:
is this not supposed to be `>= version 8`
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/others/TestMergeModeEventTimeOrdering.scala:
##########
@@ -32,19 +32,23 @@ import
org.apache.spark.sql.hudi.common.HoodieSparkSqlTestBase.validateTableConf
class TestMergeModeEventTimeOrdering extends HoodieSparkSqlTestBase {
// TODO(HUDI-8938): add "mor,6,true", "mor,6,false" after the fix
- Seq("cow,8,true", "cow,8,false", "cow,6,true", "cow,6,false",
- "mor,8,true", "mor,8,false").foreach { args =>
+ Seq("cow,current,true", "cow,current,false", "cow,6,true", "cow,6,false",
+ "mor,current,true", "mor,current,false").foreach { args =>
Review Comment:
can we include below combos as well
cow, eight, true
mor, eight, true
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestMORDataSource.scala:
##########
@@ -226,7 +247,7 @@ class TestMORDataSource extends HoodieSparkClientTestBase
with SparkDatasetMixin
// incremental view
// validate incremental queries only for table version 8
// 1.0 reader (table version 8) supports incremental query reads using
completion time
- if (tableVersion.equals("EIGHT")) {
+ if (tableVersion.equals("CURRENT") || tableVersion.equals("EIGHT")) {
Review Comment:
same comment as above
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestMORDataSource.scala:
##########
@@ -185,7 +205,8 @@ class TestMORDataSource extends HoodieSparkClientTestBase
with SparkDatasetMixin
Seq(HoodieTableConfig.PRECOMBINE_FIELD.key)
}).asJava
HoodieTestUtils.validateTableConfig(storage, basePath, expectedConfigs,
nonExistentConfigs)
- val commit1CompletionTime = if (tableVersion.equals("EIGHT")) {
+ val commit1CompletionTime = if (
+ tableVersion.equals("CURRENT") || tableVersion.equals("EIGHT")) {
Review Comment:
can we do tableVersion.greaterThanOrEquals () so that we don't need to fix
this in future
--
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]
