voonhous commented on code in PR #19133: URL: https://github.com/apache/hudi/pull/19133#discussion_r3563718940
########## 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: +1, this is the most valuable case to add. The whole point of the suite is to catch a legacy per-version parquet-read regression, and the vectorized nested-column path (HUDI-7190 / #10265) is where those have historically lived. Adding a struct (and ideally an array) column to `makeRows`/the schema, exercised under both the vectorized and the non-vectorized branches you already toggle, would close that gap. The flat-column cases here wouldn't have caught #10265. -- 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]
