fix: apply Spark's Parquet conversion rules to nested struct/list/map fields

[peterxcli]

Which issue does this PR close?

Closes #5671.

Rationale for this change

Comet must apply Spark's Parquet conversion rules to every requested field, including fields inside structs, arrays, and maps. Previously, validation stopped at the outer type. Nested conversions could return null on overflow, parse or stringify values that Spark rejects, or panic when a converted array had the wrong type.

What changes are included in this PR?

The schema adapter now walks nested fields and applies the existing scalar conversion rules to each leaf. Validation and conversion use the same field matching rules, including field IDs and case sensitivity. Missing fields continue to read as null or their configured defaults. Conversions that Spark rejects only when reading a row group retain that behavior for empty files.

The walk accepts Arrow List, LargeList, and FixedSizeList representations of a Spark array and checks their element types. It unwraps physical dictionary types before choosing how to walk a container, and rejects malformed map entries or incompatible map ordering flags. Unsupported runtime conversions return an error, and array construction uses checked constructors.

Conversion errors include the nested column path. For Spark's standard list encoding, the path includes the repeated list group, such as [a, list, element, x]. Arrow's schema omits the original repeated group name, so paths from legacy list encodings or custom group names can still differ. This limitation is documented in the scan compatibility guide.

Iceberg applies schema promotion before these checks. The pinned iceberg-rust reader passes batches through RecordBatchTransformer, which casts changed types to the requested table schema before Comet's adapter receives them. This ordering preserves Iceberg's nested numeric promotions.

How are these changes tested?

Rust regressions write and scan real Parquet files through the schema adapter. They cover rejected nested conversions, valid widening, missing fields, empty files, list offset conversion with null and empty values, and LargeList element narrowing. Focused schema checks cover dictionary-wrapped containers and invalid map shapes.

The Scala rejection test now requires a native Comet scan in the executed plan before collecting results. It checks all nine rejection cases and verifies the error path for an array of structs. Companion tests cover Spark's version-dependent widening rules and nested timestamp units.

Local validation after the review fixes:

• cargo test --release --locked -p datafusion-comet parquet: 133 passed, 1 failed, 4 ignored. All four added regressions passed. The failure was the unchanged S3 test_cached_credential_provider_refresh_credential timing test; it passed alone and failed again in a serial Parquet run.
• ParquetReadV1Suite nested on Spark 4.1.3: all 6 selected tests passed, including the nine rejection cases with the native scan assertion.
• Native release build, JVM compilation, cargo fmt --all --check, Spotless, Prettier, and git diff --check passed.

The Iceberg promotion ordering was verified in the pinned source; no new Iceberg runtime test was run.

Fixed-size list review follow-up

Commit f82207739 extends the existing array eligibility walk to FixedSizeList and keeps the complex-type classification and Spark catalog names consistent. Two real Parquet regressions cover nested fixed-size arrays read as List, LargeList, or FixedSizeList: unchanged Int64 values and nulls are preserved, while Int64 -> Int32 elements are rejected at [s, a, list, item].

The baseline run reproduced both the valid-read rejection and the incorrect container-level narrowing error. After the fix, all 67 schema adapter tests passed (zero failures), including both new regressions. Rust formatting and whitespace checks passed. The final correctness run used release dependencies with temporary package overrides for the core crate (opt-level=0, codegen-units=16) to avoid repeating the expensive optimized build. These overrides are command-line settings, not repository changes.

[sunchao]

Summary

The reader previously applied these Spark conversion restrictions at the top level while nested struct, list and map fields could reach a more permissive conversion path. This patch applies the leaf policy recursively, shares field matching between validation and conversion, and replaces unchecked nested construction with recoverable errors.

Correctness and compatibility

I checked the maintained Spark 3.5 and 4.0 reader sources, including leaf conversion, field-ID precedence, missing fields, case matching and nested shape handling. I also checked the distinction between an empty file and a nonempty file containing empty or null collections. The recursive walk and fallback validation are consistent with those paths. This remains a partial conversion-rejection policy, not a claim that every Arrow conversion matches every Spark version.

Validation and CI

The review's component validation passed 32 base tests, 47 head tests and eight additional real-Parquet probes. These use the production adapter and conversion code with matching dependency versions. They are not a full native-core build, local planner-test run or Spark/JVM execution. The fixture reproduction uses ArrowWriter and DataSourceExec, while the existing planner helper writes through write_parquet.

At 14:36 UTC, CI had 59 successful checks, eight skipped, five running and one failure. The Rust job failed on test_nested_types_list_of_struct_by_index. Its actual checkout is synthetic merge 5338cef07e79101552ae84ca3b0ab8caf4f492d0, with the exact reviewed base/head parents and matching relevant source. The inline P2 describes the fixture change needed to keep that test valid under the stricter rule.

Performance

Validation walks the schema rather than individual rows. Shared struct matching adds an index vector proportional to the requested fields for each converted struct batch, but does not copy child data buffers solely to validate their types. I found no demonstrated P1/P2 performance regression. No throughput or memory improvement was measured, and no benchmark was run.

Design

Using one field resolver for validation and conversion avoids validating one field while reading another after field-ID matching, renaming or reordering. Applying the policy to both normal rewrites and the fallback closes the bypass without introducing a separate conversion framework. The deferred rejection preserves the relevant empty-file behavior.

Abstraction & complexity

One recursive walker and the Accept/Reject/RejectOnNonEmpty distinction fit the policy being enforced. Shared matching removes duplicated selection logic, while checked constructors keep invalid nested arrays on the error path. No additional actionable concern was verified in these areas.

[sunchao]

Correctness

The previous [P2] finding is fixed by e4411074. CAST(1 AS INT) makes the fixture's nested a field an Int32, matching the unchanged required schema. The test still projects a and c and expects [{a: 1, c: x}].

I checked DataFusion's literal/cast type resolution and the maintained Spark 3.5 and 4.0 reader rules. Both Spark versions accept Parquet INT32 as int and reject INT64 as int for this nested leaf, even when the value fits. Explicitly typing the fixture preserves that production restriction. The reader implementation, nullability, field matching, empty-file handling and rejection rules are unchanged from the previous head. No new P1/P2 was found in this update.

Validation and CI

The Rust CI job passed test_nested_types_list_of_struct_by_index and finished with 1,127 tests passed and four skipped, including the nested long-to-int rejection regressions. Its actual checkout was merge ae450fbd, whose parents are the exact reviewed base 81d637b9 and head e4411074. The relevant fixture and reader code match the reviewed source.

At 16:50 UTC, the current-head check read reported 1 failed, 27 in progress, 7 skipped, 36 successful. The Spark 4.1 SQL/Hive shard failed during sbt project loading when Maven Central returned HTTP 502 for org.junit:junit-bom:5.9.3. That shard did not reach test execution. I did not rerun CI, and this is not an all-green CI result. Local validation in this follow-up comprised 19 source/provenance assertions, not runtime component tests. I did not rerun native or Spark/JVM tests locally.

Performance

This update changes only test-data construction. It adds no production scan work, allocation or copying. The stricter conversion path is unchanged, so this correction introduces no new production performance mechanism to benchmark. No benchmark was run.

Design

The explicit fixture type keeps the test focused on nested field projection while preserving the Parquet conversion policy. The required schema and expected result remain intact. This is a direct correction to the test input rather than an exception in the reader.

Abstraction & complexity

The correction adds no helper, special case or configuration. It expresses the intended type directly in the fixture SQL and leaves the existing validation and conversion boundaries unchanged. No further abstraction change is needed for this fix.

[andygrove]

Thanks for taking this on. The approach is right and the analysis in the description is careful.

I traced SparkPhysicalExprAdapterFactory and confirmed spark_parquet_convert is only reachable through CometCastColumnExpr, which only the schema adapter builds, so turning the _ => Ok(array) fallthrough into an error is well contained. The rule extraction into check_leaf_conversion really is a no-op for the top-level path, same conditions in the same order. I also checked that running check_conversion before is_pure_structural_narrowing is safe, since that predicate requires exact leaf matches so every leaf hits the physical_type == target_type early accept.

I verified the error string against Spark too. parquet_schema_convert_err produces [a], and Spark's constructConvertNotSupportedException passes Arrays.toString(descriptor.getPath()), which is also [a]. The extra bracket pair in the Rust-level Display never reaches the JVM shim, so the final SchemaColumnConvertNotSupportedException matches. That is easy to misread, so it is worth stating.

The test suite is genuinely good. The Rust tests run through a real DataSourceExec with the real adapter factory rather than calling the checker directly, they cover all six cases from the issue including the former panic, and the positives (nested_int_widening_succeeds_with_type_promotion, nested_timestamp_millis_read_as_micros_succeeds, nested_missing_field_reads_as_null, nested_disallowed_widening_passes_for_empty_file) are the right guards against over-rejection.

A few things I would like to see addressed.

1. The list column path drops Spark's repeated-group segment

check_conversion's list arm (schema_adapter.rs:739-744) appends only the Arrow item field name. Spark's ParquetSchemaConverter writes a 3-level list as repeated group list { element } (ParquetSchemaConverter.scala:873-875), so descriptor.getPath() for a array<struct<x>> is ["a", "list", "element", "x"] and the exception reads Column: [a, list, element, x]. This produces Column: [a, element, x].

That is not cosmetic in the way it might look. The string is passed verbatim into SchemaColumnConvertNotSupportedException by ShimSparkErrorConverter, so it is what users see when they compare Comet against Spark. The map arm just below does insert the repeated-group name (key_value), which matches ParquetSchemaConverter.scala:893 exactly, so the two arms are inconsistent with each other. nested_list_of_struct_long_read_as_int_errors currently locks in [a, element, x].

Could the list arm append a synthetic list segment the way the map arm does? If Arrow genuinely cannot recover the group name in every case, could the doc comment on check_conversion and that test say so, and could compatibility/scans.md get an entry, given that page describes its list as the remaining gaps?

2. LargeList is rendered but not gated

spark_catalog_name at schema_adapter.rs:378 renders LargeList as array<...>, but neither check_conversion's walk nor the is_complex closure at schema_adapter.rs:691 knows about it. Does that mean a LargeList(Int64) file column read as LargeList(Int32) gets accepted here, falls through to can_cast_types in parquet_convert_array_impl with safe: true, and silently NULLs on overflow, which is the bug this PR fixes for List?

I do not think the arrow-rs Parquet reader emits LargeList today, so this looks latent rather than live. Since this is a correctness gate it would be good for it to cover the same set of shapes spark_catalog_name does.

A related shape gap in the same area: check_conversion's map arm accepts unconditionally when the entries are not a pair of structs, and it ignores the sorted flag while parquet_convert_array_impl's map arm requires ordered_from == ordered_to. A mismatch there now surfaces as the new generic Unsupported Parquet type conversion execution error rather than a Spark-shaped rejection.

3. Dictionary unwrapping happens after the shape dispatch

The Dictionary(_, value) unwrap lives at the top of check_leaf_conversion (schema_adapter.rs:507), but check_conversion dispatches on shape before it ever runs. So a Dictionary(_, Struct{a,b}) physical column read as Struct{a} never gets walked. It falls to the leaf check, unwraps to a struct, fails the equality test, and hits the blanket complex reject at schema_adapter.rs:691, which would be a false rejection of an ordinary pruning read. Same story for a dictionary-wrapped list or map. Would moving the unwrap to the top of check_conversion work, so the walk and the leaf rules both see the same normalized type?

4. Does this touch the Iceberg scan path?

iceberg_scan.rs:232-233 builds a SparkPhysicalExprAdapterFactory from a bare SparkParquetOptions::new(EvalMode::Legacy, "UTC", false), which defaults allow_type_promotion and allow_timestamp_ltz_to_ntz to false regardless of Spark version. Iceberg's own schema evolution legitimately permits int -> long and float -> double promotion, including on nested fields.

Does iceberg-rust's ArrowReader resolve all nested promotions before the batch reaches the adapter? If it does, this is a non-issue and a sentence in the description is enough. If it does not, a nested Iceberg promotion that used to be cast would now be rejected, and I do not think the Iceberg suites would catch it.

5. The Scala regression test can pass vacuously

native scan rejects nested Parquet conversions Spark rejects asserts that both engines throw and that Comet's cause chain contains SchemaColumnConvertNotSupportedException. If Comet fell back to Spark's scan for one of those nine cases, Spark's own exception satisfies both assertions and the case contributes no coverage. That risk seems higher for nested read schemas than for the top-level tests this one is modeled on. Would it be worth asserting that the executed plan actually contains the Comet scan before collecting?

None of this is an objection to the approach. Items 1 through 3 are contained fixes in check_conversion, item 4 may only need a sentence in the description, and item 5 is a few lines in the test.

[sunchao]

Correctness

The 161c2076 merge keeps the earlier fixture correction and places check_conversion before the DataFusion 55 structural-pruning shortcut. Both checking functions are unchanged from e4411074. The shortcut still requires matching leaf types, so it does not bypass the nested numeric, decimal or LTZ-to-NTZ restrictions.

I rechecked the maintained Spark 3.5 and 4.0 readers. Both reject nested INT64-to-int and numeric-to-string reads. Spark 4.0 permits the widening pairs gated off for 3.5. The shared resolver preserves field-ID precedence, case-insensitive matching and missing-field handling. The conversion retains parent and collection validity buffers. A nonempty batch still triggers deferred rejection even when its parent values are null or its collections are empty.

[P2] Existing LargeList concern is reachable

Following up on item 2 in Andy's review, Parquet 59.3.0 does preserve LargeList from ARROW:schema. A nested file field s.a: LargeList<Int64> read as Spark s.a: array<bigint> reaches the new leaf check as LargeList<Int64> -> List<Int64>. It is rejected because the walker recognizes only List/List, then the shape guard recognizes the target List. The element type has not changed. Spark reads the underlying Parquet LIST normally, and the base nested converter supports this offset-width conversion. DataFusion's file-schema coercion does not normalize it away.

Could the existing list-shape fix cover this valid nested read as well as rejecting disallowed element conversions? This is source-traced against the locked dependencies, not a newly executed reproduction. I am keeping this in the existing discussion rather than adding a duplicate inline.

I also confirmed the list-path omission and the missing native-plan assertion identified in items 1 and 5 of that review. For item 4, the pinned iceberg-rust reader applies RecordBatchTransformer before returning batches. It selects a promotion when the requested type differs and casts to that target before Comet's adapter runs. I found no additional Iceberg promotion regression in that path. I have not established a reachable dictionary-of-container or malformed-map regression.

Validation and CI

At 02:56:19 UTC on September 8, the complete snapshot contained 64 successful checks and 51 skipped checks, with no failures or running checks. The Rust job passed 1,183 tests with four skipped, including the previous fixture, nested rejection/acceptance cases and structural-pruning guards. All three new Scala cases passed in the Linux 3.5 scan job, Linux 4.0 scan job and macOS 4.0 scan job.

Those jobs checked out merge f3b53378, with the exact assigned base/head parents and an entire tree equal to this head. The macOS native artifact matches its producer's ID and digest. The Linux consumers used a different artifact ID/digest from the inspected native-build upload. Both artifacts belong to this head and workflow, but that producer-to-consumer link remains unverified. The Scala rejection helper also permits fallback, so its passing result does not independently prove native execution for each rejection case. I did not run local native/JVM tests or benchmarks. Maintained Spark 3.4 and 4.1 sources remain unavailable.

Performance

The merge retains DataFusion's pruning shortcut for matching struct/list leaves after validation. The additional walk operates on schemas, while shared struct matching allocates an index vector for conversion. It does not copy child buffers merely to validate their types. I found no further actionable performance regression. CI correctness results do not establish scan throughput or memory gains.

Design

Running the rejection policy before selecting the conversion implementation remains the right order. The outstanding list issue is at that policy boundary: Arrow's offset-width variants must be distinguished from incompatible Spark shapes. The Iceberg transformer already provides a separate normalization boundary, so the inspected path does not need a new adapter framework.

Abstraction & complexity

The shared field resolver and three conversion outcomes remain focused abstractions. The merge does not duplicate the scalar rules around the pruning shortcut. Extending the existing list handling can address the confirmed concern without adding a second recursive validation system.

[sunchao]

Correctness

Follow-up on 37d83193afd7625213813734727098013ab2ff5f, against base 7e1984399eb887cd13109698ee55cf2ce150f849. The update fixes the exact LargeList case from the previous review. Dictionary unwrapping now precedes shape dispatch, map validation checks the runtime converter's shape requirements, and the Scala rejection test asserts a native scan before collecting. The standard list error path now includes list; the documented legacy/custom-name limitation is accurate.

[P2] Accept the Parquet-preserved fixed-size list representation

The list arm still accepts only List and LargeList. A nonempty Parquet file with Arrow schema metadata for s: struct<a: FixedSizeList<Int64, 2>>, containing [1, 2], is a valid input when Spark requests s: struct<a: array<bigint>>. Parquet 59.3 preserves that fixed-size list hint and has a reader for it; DataFusion's file-schema coercion does not normalize it inside this struct. The new walker reaches check_leaf_conversion(FixedSizeList, List) and rejects it as a shape mismatch. At the assigned base, the enclosing struct reaches Comet's recursive converter, whose Arrow cast supports this fixed-size-list-to-list conversion. This is a new concrete case in the existing array-representation discussion. Please walk the fixed-size list's element type as well, with a real Parquet regression covering the unchanged-element case and a prohibited leaf conversion. This finding is established from the pinned reader, adapter and cast sources; I did not execute this fixture locally.

The inspected Rust job passed 1,209 tests with four skipped, including all four new Rust tests. The Linux Spark 4.2 scans job passed the three nested Scala tests; its totals were 257 succeeded, zero failed, 233 canceled and one ignored. The native artifact ID and digest now agree between the producer and this consumer. These jobs checked out merge 4fc635d41f675cb2aafdbbfacac543b094512eac, with newer-main parent 99d3100c60cdf5c1fae26ac7d7102953b250900f; all five PR files and the checked scan/dependency sources match the reviewed head, but the whole merge tree differs. At the September 8, 05:03:50 UTC refresh, 51 checks succeeded, seven were skipped, 13 remained in progress and one failed. The failed Iceberg extensions job ended with context canceled and an explicit runner shutdown signal; that interruption is not a verified code finding, and I do not count the interrupted suite as passed. Canonical Spark 3.5/4.0 sources were checked; maintained 3.4/4.1 sources were unavailable. No local native/JVM execution or new Iceberg runtime validation was performed.

Performance

The additional eligibility work runs when adapting the schema, rather than for each row. The list-width tests cover preserved values and null/empty containers for the supported List/LargeList pairs, but they do not measure conversion cost. I found no additional verified performance regression in this update; there is no measured speedup claim.

Design

Unwrapping dictionaries before dispatch and sharing struct-field resolution keep validation aligned with conversion. The remaining P2 is an incomplete list-family dispatch: extending the existing recursive arm can retain the leaf restrictions without introducing a separate conversion policy. The Iceberg promotion path is unchanged in this update, and the earlier source trace still places its schema transformer before Comet conversion.

Abstraction & complexity

The changes remain localized to the existing walker, documentation and regression tests. The native-plan assertion makes the Scala rejection test meaningful, while the dictionary/container and malformed-map tests validate schema decisions only; they do not demonstrate every accepted runtime representation. I have no separate abstraction finding beyond completing the array family above.

[sunchao]

Correctness

Follow-up on f822077392ea35cc606458c3b116059cb8b89319 against 7e1984399eb887cd13109698ee55cf2ce150f849. The concrete FixedSizeList case in the previous review is fixed. Its element now reaches the existing leaf policy. The two new real-Parquet tests check preserved Int64 values, null elements and null lists across three target representations, and verify that narrowing reports the leaf path and INT64 -> int mismatch. This agrees with the maintained Spark 3.5 and 4.0 readers for these leaf types.

[P2] Include both Parquet list-view representations

The updated array arm still omits ListView and LargeListView. Both can occur in a valid Parquet file. For example, write s: struct<a: ListView<Int64>> with offsets [0], sizes [2], values [1, 2] and the default Arrow schema metadata, then request Spark s: struct<a: array<bigint>>. The same example applies to LargeListView with 64-bit offsets and sizes.

I traced both cases through the locked Parquet 59.3 writer, metadata conversion and reader. The writer supports both forms, the schema hint is preserved, and ListViewArrayReader::consume_batch returns a view array. DataFusion 55 leaves this nested type intact. Spark reads the underlying standard LIST/INT64 encoding normally. At the assigned base, Comet's enclosing struct converter reaches Arrow's supported cast_list_view_to_list path. Here, check_conversion falls through to the leaf check and rejects the complex target List before that conversion can run. This is one remaining array-representation issue covering both view types. Please include both in the recursive check, complex classification and catalog naming, with real-Parquet tests for an unchanged element type and prohibited narrowing. This conclusion is source-verified, not a locally executed reproduction.

The current Rust CI job passed both new tests and finished with 1,211 passed and four skipped. It checked out merge 32b14e2be511d82ce7425749606634acb8522e42, whose parents are newer main 6368c4e6190f7d569fcbd57c71bdb15dc9c5d33b and this head. The 17 checked relevant files, including all five PR files and the dependency lock, match the head. The whole merge tree differs in 33 files. At the September 8, 06:34:10 UTC refresh, 31 checks succeeded, seven were skipped and 34 were running, with no failures. I have not independently verified current Spark scan or Iceberg suite execution and native-artifact consumption, so I am not crediting those results here. I ran only source, whitespace and formatting checks locally. Maintained Spark 3.4/4.1 sources remain unavailable.

Performance

This update adds FixedSizeList to schema matches and error-name rendering. It introduces no per-row loop or data-buffer copy in the eligibility check. I found no additional verified performance regression, and no throughput or memory improvement was measured.

Design

The FixedSizeList correction uses the existing recursive policy and preserves prohibited leaf conversions. Dictionary unwrapping, map guards and the Scala native-plan assertion remain unchanged. Completing the two remaining view variants in the same policy would address the confirmed issue without adding another normalization layer.

Abstraction & complexity

The two focused tests exercise the real reader and adapter through the existing helper. The repeated array-family matches should cover the same supported representations, as the remaining omission demonstrates. Completing those matches is sufficient for this fix. I found no separate abstraction issue requiring another helper or configuration.

[sunchao]

Correctness

Follow-up on a7180398227c7c4894f6ae8c920e38965f907f1f against 7e1984399eb887cd13109698ee55cf2ce150f849. The combined ListView/LargeListView P2 in the previous review is addressed. Both representations now participate in array naming, complex-type classification and recursive element validation. An unchanged Int64 element reaches the existing converter, while Int64 -> Int32 still fails at the leaf. This agrees with the maintained Spark 3.5 and 4.0 readers for standard Parquet LIST/INT64 encoding. Dictionary unwrapping, map guards, promotion flags and both adapter rewrite paths retain their existing checks. I found no remaining verified P1/P2 issue in this follow-up.

The new nested_list_view_conversion test writes actual Parquet with both view types and reads it through the adapter. It compares the resulting schema and values against the original ordinary list, covering values beyond the Int32 range, null elements, null lists and empty lists. It also checks six prohibited narrowing combinations and their full leaf error path. The existing empty-file behavior is unchanged. Sliced-array behavior was traced through the locked Arrow slice/cast and Parquet writer/reader sources. This update adds no sliced fixture, overlapping-view fixture or null-parent-struct fixture, and I did not execute those cases locally.

Validation

The Rust job passed the new test and finished with 1,216 passed and four skipped. The Linux scan jobs for Spark 3.5 and Spark 4.0 passed all three named nested-conversion, widening and TIMESTAMP_MILLIS tests. Their suite totals were 475 passed/15 canceled/one ignored and 482 passed/eight canceled/one ignored, respectively, with no failures. Both downloaded native artifact 10048397811. Its producer, metadata and consumer SHA256 digests match.

These jobs ran merge 93bf90ecc259c91798a38ae7bbbe961b91cdc64e, whose parents are newer main bb9e74020adc228e486f6f4d0fa68292b30bff31 and this head. All 17 checked relevant files, including the five PR files and dependency lock, match the head. The whole merge tree differs in 34 files. At September 8, 09:38:33 UTC, 50 checks had succeeded, seven were skipped and 14 were running, with no failures. This does not establish completion of the broader Spark SQL or Iceberg suites. Local validation was limited to source, whitespace and formatting checks. Maintained Spark 3.4/4.1 sources remain unavailable. No full source-parity claim is made for those versions.

Performance

The incremental production change extends schema matches and error-name rendering. It adds no row loop or data-buffer copy to eligibility checking. View-to-list conversion retains the existing Arrow conversion and copying behavior. I found no new material performance issue, and no throughput or memory improvement was measured.

Design

Completing the existing recursive array policy resolves the representation mismatch while preserving Spark's leaf restrictions. It avoids another normalization layer, leaves structural pruning unchanged and keeps the Scala native-plan assertion in place. No further design change is needed for the verified issue.

Abstraction & complexity

The existing array-family matches now cover all five Parquet-supported list representations consistently. The focused regression uses the existing real-reader helper and compares its positive result with the original input. It adds no new helper, configuration or production abstraction requiring a separate change.

[andygrove]

Thanks for working through all of this. All five items from my earlier review are addressed and I traced each one.

The list arm now emits the repeated list segment, and the Scala test locks in [s, list, element, x], which is what Spark produces for a three-level list. The scans.md note about legacy and custom group names is the right caveat to document. is_complex and the recursive walk now cover the whole array family, and switching from is_complex(physical) != is_complex(target) to || also closes the case where a Struct read as a Map slipped through because both sides were complex. Dictionary unwrapping happens at the top of check_conversion before the shape dispatch, so a dictionary-wrapped struct gets walked instead of falsely rejected. The map arm now checks the entries shape and the sorted flag against what parquet_convert_map_to_map actually requires.

On the Iceberg question I confirmed it myself rather than take the description on faith. IcebergStreamWrapper::poll_next builds the adapter from batch.schema(), so the adapter only ever sees a batch that iceberg-rust has already run through RecordBatchTransformer. Iceberg's own nested promotions are resolved before Comet's rules apply. CometIcebergNativeSuite and CometFuzzIcebergSuite also run in the scans bucket, so that path is exercised on this PR even though the dedicated Iceberg jobs are label-gated.

The native-plan assertion makes the rejection test real, and the two positive companions (per-version widening and nested TIMESTAMP_MILLIS) are exactly the guards I wanted against over-rejection.

Two things left. The branch conflicts with main now that #5715 landed. It is two small hunks, and the one needing a judgement call is in wrap_all_type_mismatches. Main widened the condition to logical_field.has_valid_extension_type::<VariantType>() || logical_field.data_type() != physical_field.data_type(), and you want to keep that condition together with your check_conversion body rather than taking one side. I walked the Variant storage shapes through check_conversion and they all come back Accept, including a shredded layout where value is absent from the file, so I do not expect a false rejection. It would still be worth running the Variant scan tests after the rebase. The replace_with_spark_cast side auto-merges with the Variant early return ahead of check_conversion, which is the order you want.

The second is a follow-up rather than something to change here. check_conversion now accepts LargeList, FixedSizeList and the two view types, but replace_with_spark_cast's dispatch and parquet_convert_array_impl's list arm still only recognise (List, List), so those representations convert through Arrow's generic cast instead of Comet's converter. For the leaf element types your tests cover, the two agree. They diverge when the element is a struct and a requested field is absent from the file. Comet null-fills, while Arrow's cast_struct_to_struct gives up on name matching and falls back to positional, handing back the wrong column's values. Reaching it needs an arrow-rs or pyarrow written file using a non-default list representation, so it is remote, and for the nested case it predates this PR. Could you file an issue so it does not get lost?

CI is green with 64 passing and no failures. Approving.

[andygrove]

@peterxcli could you fix merge conflict?

[sunchao]

Correctness

Follow-up on 5b0f78ca46cf7b68f1e1c1f4b6a13e38d4a41378 against 17f54da8ca5cb0ad5dbe8357b6e037fef8a0db2c. The merge preserves the Variant condition in wrap_all_type_mismatches together with recursive validation, and keeps Variant normalization ahead of ordinary cast replacement. The inherited ambiguity test now expects the early duplicate-field error, consistent with the maintained Spark 3.5/4.0 name resolver. The earlier fixture and list-view findings remain addressed.

[P2] Preserve field matching for newly admitted top-level lists

Following up on the generic-cast concern in Andy's review, there is a top-level case that this PR newly admits. Write a Parquet file with Arrow metadata for a: LargeList<struct<old:bigint, keep:bigint>>, containing a = [{old:7, keep:2}], then request Spark a array<struct<missing:bigint, keep:bigint>>, with nullable fields and field-ID matching disabled. Spark 3.5/4.0 match the children by name and return [{missing:null, keep:2}].

At the assigned base, the normal adapter rejects this LargeList -> List pair at its scalar/complex mismatch guard. The new array arm accepts it: it skips the missing child and accepts the unchanged keep leaf. But replace_with_spark_cast handles only List/List through the complex converter, so this pair reaches Spark Cast's schema-adaptation fallback and Arrow's generic cast. Arrow 59.3's list-width conversion casts the struct values. When any requested name is absent, its struct cast falls back to position. It therefore assigns old's 7 to missing. The projection analyzer retains the whole root, so it does not remove old first.

The enclosing-struct version discussed earlier predates this PR. This top-level pair was previously rejected. Please preserve Spark's nested field matching in the runtime route for the admitted list representations, with a real-Parquet missing-field regression. Switching only the adapter wrapper is insufficient while the Parquet converter's recursive list branch also handles only List/List. This is verified from the pinned reader, adapter, projection and cast sources. I did not execute a runtime reproduction. I am keeping the follow-up in this discussion without another inline.

Validation

The Rust job passed 1,274 tests, five skipped, including the list-view, ambiguity and Variant normalization tests. Spark 3.5 scans passed 483 tests/16 canceled/one ignored. Spark 4.0 scans passed 490/nine canceled/one ignored. Both had zero failures and passed all three named nested-conversion tests. Their native artifact ID 10087696150 and SHA256 match the producer and metadata.

All four inspected jobs checked out 38470cc56edc69442781c3b549ca9fb1b28e024e, with the exact assigned base/head parents and a tree identical to this head. At September 9, 05:10:13 UTC, 63 checks succeeded, eight were skipped and two were running, with no failures. The missing-field/non-default-list combination is absent from the current fixtures. Local checks were source, whitespace and formatting only. Maintained Spark 3.4/4.1 sources remain unavailable. No parity claim is made for those branches.

Performance

The merge keeps schema-level validation and the shared matching vector. It adds no new per-row conversion loop. The correctness fix should retain existing offset/null buffers where possible while selecting struct children by Spark's resolver. No throughput or memory benchmark was run, and no measured gain is claimed.

Design

The Variant conflict resolution preserves both responsibilities. The remaining P2 is a mismatch between the representations admitted by validation and the field semantics used by their runtime converter. Extending the existing recursive conversion path can align those responsibilities without a separate normalization framework.

Abstraction & complexity

The shared resolver and three conversion outcomes remain appropriate. The missing-field case shows why every admitted list family must reach that resolver at conversion time as well. Reusing it is sufficient. Another field-matching helper or configuration would add unnecessary complexity.