voonhous commented on code in PR #19133: URL: https://github.com/apache/hudi/pull/19133#discussion_r3565830655
########## hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestLegacyParquetReadPath.scala: ########## @@ -0,0 +1,292 @@ +/* + * 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, assertFalse, assertTrue} + +/** Row shape written by these tests. A nested struct and an array are included so the legacy + * parquet read path is exercised on complex types -- the historically fragile vectorized + * nested-column branch (e.g. HUDI-7190), not just flat scalar columns. */ +private case class LegacyNested(a: Int, b: String) + +private case class LegacyTestRow(id: String, + ts: Long, + value: Long, + partition: String, + nested: LegacyNested, + tags: Seq[Int]) + +/** + * 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 { Review Comment: Agreed, good catch -- every case round-tripped the same schema, so typeChangeInfos stayed empty and the read never left stock VectorizedParquetRecordReader. Added testCowSnapshotReadWithImplicitTypeChange: commit 1 writes `value` as int32; commit 2 upserts only the p0 rows (id % 3 == 0) with a long value > Int.MaxValue, promoting the table schema to long. Updating a single partition is deliberate -- upserting ids 1-10 would rewrite all three partitions' file groups and leave no narrow base files, whereas this keeps the p1/p2 base files physically int. Reading them against the long table schema now runs through buildImplicitSchemaChangeInfo and HoodieVectorizedParquetRecordReader (which widens int->long via convertIntLongType). The test asserts the vectorized branch is actually engaged (not the parquet-mr fallback), that the values widen correctly, and that the legacy path still matches the file-group reader. Passes on spark3.5. One nuance: the implicit path does not hit the InternalSchemaCache.getInternalSchemaByVersionId lookup -- that only fires on the explicit schema-on-read path (HOODIE_QUERY_SCHEMA set), which needs hoodie.schema.on.read.enable plus an ALTER. Happy to add a separate case for that branch if you want it covered here, otherwise Ill leave it as a follow-up. -- 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]
