voonhous commented on code in PR #19133: URL: https://github.com/apache/hudi/pull/19133#discussion_r3563750460
########## hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestLegacyParquetReadPath.scala: ########## @@ -0,0 +1,246 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.functional + +import org.apache.hudi.{BaseFileOnlyRelation, DataSourceReadOptions, DataSourceWriteOptions, IncrementalRelationV1, IncrementalRelationV2} +import org.apache.hudi.common.config.HoodieReaderConfig +import org.apache.hudi.common.table.HoodieTableConfig +import org.apache.hudi.common.table.log.InstantRange.RangeType +import org.apache.hudi.config.HoodieWriteConfig +import org.apache.hudi.testutils.HoodieSparkClientTestBase + +import org.apache.spark.sql.{DataFrame, Row, SaveMode, SparkSession} +import org.apache.spark.sql.functions.col +import org.junit.jupiter.api.{AfterEach, BeforeEach, Test} +import org.junit.jupiter.api.Assertions.{assertEquals, assertTrue} + +/** + * Functional tests for the legacy (pre-file-group-reader) Spark read path: + * [[BaseFileOnlyRelation]], [[IncrementalRelationV1]], [[IncrementalRelationV2]] and the + * per-Spark-version legacy Hudi parquet file format created via + * `sparkAdapter.createLegacyHoodieParquetFileFormat`. + * + * In the batch datasource, `DefaultSource` routes normal reads to the file-group-reader-based + * relations regardless of `hoodie.file.group.reader.enabled`; the legacy relations still run in + * production for metadata-table reads and for streaming reads with the flag disabled. To exercise + * them functionally here, the legacy relations are constructed directly (with the flag set to + * false in their options, matching how the streaming sources invoke them) and their results are + * compared row-by-row against the file-group-reader-enabled reads of the same table. + */ +class TestLegacyParquetReadPath extends HoodieSparkClientTestBase { + + var spark: SparkSession = _ + + private val writeOpts = Map( + "hoodie.insert.shuffle.parallelism" -> "2", + "hoodie.upsert.shuffle.parallelism" -> "2", + DataSourceWriteOptions.RECORDKEY_FIELD.key -> "id", + DataSourceWriteOptions.PARTITIONPATH_FIELD.key -> "partition", + DataSourceWriteOptions.TABLE_TYPE.key -> DataSourceWriteOptions.COW_TABLE_TYPE_OPT_VAL, + DataSourceWriteOptions.HIVE_STYLE_PARTITIONING.key -> "true", + HoodieTableConfig.ORDERING_FIELDS.key -> "ts", + HoodieWriteConfig.TBL_NAME.key -> "legacy_read_path_tbl" + ) + + // Columns compared across the read paths; meta fields are persisted in the base files, so the + // record key and commit time must match exactly between the legacy and new readers. + private val comparedCols = Seq("_hoodie_commit_time", "_hoodie_record_key", "id", "ts", "value", "partition") + + @BeforeEach override def setUp(): Unit = { + setTableName("legacy_read_path_tbl") + initPath() + initSparkContexts() + spark = sqlContext.sparkSession + initHoodieStorage() + } + + @AfterEach override def tearDown(): Unit = { + cleanupResources() + spark = null + } + + private def makeRows(ids: Seq[Int], ts: Long, valueFn: Int => Long): Seq[(String, Long, Long, String)] = Review Comment: Addressed in 60aabbf: added a `nested` struct and a `tags` array to the shared row shape, so every snapshot case now drives the vectorized nested-column read, and `testCowSnapshotReadWithoutVectorizedReader` covers the row-based branch. The nested/array values are derived from value/ts, so upserted rows are compared with changed nested content too, not just inserts. ########## hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestLegacyParquetReadPath.scala: ########## @@ -0,0 +1,246 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.functional + +import org.apache.hudi.{BaseFileOnlyRelation, DataSourceReadOptions, DataSourceWriteOptions, IncrementalRelationV1, IncrementalRelationV2} +import org.apache.hudi.common.config.HoodieReaderConfig +import org.apache.hudi.common.table.HoodieTableConfig +import org.apache.hudi.common.table.log.InstantRange.RangeType +import org.apache.hudi.config.HoodieWriteConfig +import org.apache.hudi.testutils.HoodieSparkClientTestBase + +import org.apache.spark.sql.{DataFrame, Row, SaveMode, SparkSession} +import org.apache.spark.sql.functions.col +import org.junit.jupiter.api.{AfterEach, BeforeEach, Test} +import org.junit.jupiter.api.Assertions.{assertEquals, assertTrue} + +/** + * Functional tests for the legacy (pre-file-group-reader) Spark read path: + * [[BaseFileOnlyRelation]], [[IncrementalRelationV1]], [[IncrementalRelationV2]] and the + * per-Spark-version legacy Hudi parquet file format created via + * `sparkAdapter.createLegacyHoodieParquetFileFormat`. + * + * In the batch datasource, `DefaultSource` routes normal reads to the file-group-reader-based + * relations regardless of `hoodie.file.group.reader.enabled`; the legacy relations still run in + * production for metadata-table reads and for streaming reads with the flag disabled. To exercise + * them functionally here, the legacy relations are constructed directly (with the flag set to + * false in their options, matching how the streaming sources invoke them) and their results are + * compared row-by-row against the file-group-reader-enabled reads of the same table. + */ +class TestLegacyParquetReadPath extends HoodieSparkClientTestBase { + + var spark: SparkSession = _ + + private val writeOpts = Map( + "hoodie.insert.shuffle.parallelism" -> "2", + "hoodie.upsert.shuffle.parallelism" -> "2", + DataSourceWriteOptions.RECORDKEY_FIELD.key -> "id", + DataSourceWriteOptions.PARTITIONPATH_FIELD.key -> "partition", + DataSourceWriteOptions.TABLE_TYPE.key -> DataSourceWriteOptions.COW_TABLE_TYPE_OPT_VAL, + DataSourceWriteOptions.HIVE_STYLE_PARTITIONING.key -> "true", + HoodieTableConfig.ORDERING_FIELDS.key -> "ts", + HoodieWriteConfig.TBL_NAME.key -> "legacy_read_path_tbl" + ) + + // Columns compared across the read paths; meta fields are persisted in the base files, so the + // record key and commit time must match exactly between the legacy and new readers. + private val comparedCols = Seq("_hoodie_commit_time", "_hoodie_record_key", "id", "ts", "value", "partition") + + @BeforeEach override def setUp(): Unit = { + setTableName("legacy_read_path_tbl") + initPath() + initSparkContexts() + spark = sqlContext.sparkSession + initHoodieStorage() + } + + @AfterEach override def tearDown(): Unit = { + cleanupResources() + spark = null + } + + private def makeRows(ids: Seq[Int], ts: Long, valueFn: Int => Long): Seq[(String, Long, Long, String)] = + ids.map(i => (i.toString, ts, valueFn(i), "p" + (i % 3))) + + private def writeBatch(rows: Seq[(String, Long, Long, String)], operation: String): Unit = { + spark.createDataFrame(rows).toDF("id", "ts", "value", "partition") + .write.format("hudi") + .options(writeOpts) + .option(DataSourceWriteOptions.OPERATION.key, operation) + .mode(SaveMode.Append) + .save(basePath) + } + + /** Commit 1: insert 30 rows; commit 2: upsert rows 1-10 with new values. */ + private def writeTwoCommits(): Unit = { + writeBatch(makeRows(1 to 30, ts = 1L, i => i * 10L), DataSourceWriteOptions.INSERT_OPERATION_OPT_VAL) + writeBatch(makeRows(1 to 10, ts = 2L, i => i * 100L), DataSourceWriteOptions.UPSERT_OPERATION_OPT_VAL) + } + + private def fgReaderDf(extraOpts: Map[String, String] = Map.empty): DataFrame = + spark.read.format("hudi") + .option(HoodieReaderConfig.FILE_GROUP_READER_ENABLED.key, "true") + .options(extraOpts) + .load(basePath) + + private def legacyReadOpts(extraOpts: Map[String, String]): Map[String, String] = + Map( + "path" -> basePath, + DataSourceReadOptions.QUERY_TYPE.key -> DataSourceReadOptions.QUERY_TYPE_SNAPSHOT_OPT_VAL, + HoodieReaderConfig.FILE_GROUP_READER_ENABLED.key -> "false" + ) ++ extraOpts + + /** + * Legacy scan through [[BaseFileOnlyRelation]]'s own `PrunedFilteredScan` implementation + * (`HoodieBaseRelation.buildScan` -> base-file readers built on the legacy parquet format). + */ + private def legacyRelationDf(extraOpts: Map[String, String] = Map.empty): DataFrame = { + val metaClient = createMetaClient(spark, basePath) + spark.baseRelationToDataFrame( + BaseFileOnlyRelation(sqlContext, metaClient, legacyReadOpts(extraOpts), None)) + } + + /** + * Legacy scan through the `HadoopFsRelation` conversion that + * `DefaultSource#resolveBaseFileOnlyRelation` applies, executing the per-Spark-version + * legacy Hudi parquet file format inside a regular file-source scan. + */ + private def legacyFileFormatDf(extraOpts: Map[String, String] = Map.empty): DataFrame = { + val metaClient = createMetaClient(spark, basePath) + val hadoopFsRelation = + BaseFileOnlyRelation(sqlContext, metaClient, legacyReadOpts(extraOpts), None).toHadoopFsRelation + assertTrue(hadoopFsRelation.fileFormat.getClass.getSimpleName.contains("LegacyHoodieParquetFileFormat"), + s"Expected the legacy parquet file format but got ${hadoopFsRelation.fileFormat.getClass.getName}") + spark.baseRelationToDataFrame(hadoopFsRelation) + } + + private def collectSorted(df: DataFrame): Seq[Row] = + df.select(comparedCols.map(col): _*).collect().toSeq.sortBy(_.getString(2).toInt) + + private def assertSameRows(expected: DataFrame, actual: DataFrame): Unit = { Review Comment: Done in 60aabbf -- `collectSorted` now sorts by `getAs[String]("id")` instead of the positional index. -- 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]
