andygrove commented on code in PR #6515:
URL: https://github.com/apache/datafusion-comet/pull/6515#discussion_r4162151362
##########
native/core/src/parquet/schema_adapter.rs:
##########
@@ -478,33 +478,79 @@ enum ConversionCheck {
},
}
-/// Apply the rejection matrix of Spark's
`ParquetVectorUpdaterFactory.getUpdater` to a single
-/// physical/logical leaf pair. `column` is the Spark-style column path used
in the error (`a`
-/// for a top-level column, `s, x` for a nested leaf, mirroring
-/// `Arrays.toString(descriptor.getPath())`). The rules and their order are
exactly those the
-/// adapter applies to top-level columns; [`check_conversion`] applies them to
nested leaves.
+/// Whether Parquet stores `data_type` as a group: a struct, a list or a map.
+fn is_complex(data_type: &DataType) -> bool {
+ matches!(
+ data_type,
+ DataType::Struct(_)
+ | DataType::List(_)
+ | DataType::LargeList(_)
+ | DataType::FixedSizeList(_, _)
+ | DataType::ListView(_)
+ | DataType::LargeListView(_)
+ | DataType::Map(_, _)
+ )
+}
+
+/// Check a pair that [`check_conversion`] doesn't walk: two primitives, or
two types of
+/// different shape. `column` is the Spark-style column path used in the error
(`a` for a
+/// top-level column, `s, x` for a nested leaf, mirroring
`Arrays.toString(descriptor.getPath())`).
+///
+/// A shape mismatch (e.g. TIMESTAMP read as ARRAY<TIMESTAMP>, or STRUCT read
as ARRAY) fails
+/// when Spark opens the file if Spark can't clip the file's type to the
requested one: a group
+/// read as another type (`ParquetToSparkSchemaConverter`), or a primitive
read as a struct, or
+/// as an array or map with a complex element
(`ParquetReadSupport.clipParquetType`). Every
+/// other pair Spark rejects, including a primitive read as an array or map of
primitives
+/// (SPARK-45604), is rejected only by `getUpdater`, which Spark calls while
decoding a row
+/// group, so the rejection is deferred to runtime (#6506).
fn check_leaf_conversion(
physical_type: &DataType,
target_type: &DataType,
column: &str,
options: &SparkParquetOptions,
-) -> ConversionCheck {
+) -> DataFusionResult<ConversionCheck> {
if physical_type == target_type {
- return ConversionCheck::Accept;
- }
- let reject = || {
- ConversionCheck::Reject(parquet_schema_convert_err(
- column,
- physical_type,
- target_type,
- ))
- };
- let reject_on_non_empty = || ConversionCheck::RejectOnNonEmpty {
+ return Ok(ConversionCheck::Accept);
+ }
+ if is_complex(physical_type) || is_complex(target_type) {
+ let is_unclipped = !is_complex(physical_type)
+ && match target_type {
+ DataType::List(item)
+ | DataType::LargeList(item)
+ | DataType::FixedSizeList(item, _)
+ | DataType::ListView(item)
+ | DataType::LargeListView(item) =>
!is_complex(item.data_type()),
Review Comment:
Fixed in 3c252caccfa954300d404b495762d667377fa6e1. Footer validation now
retains the repeated-primitive distinction in an ephemeral Arrow schema, before
the decoder loses the legacy LIST encoding. A complex requested element then
rejects at open, including empty and fully pruned files; the shared footer and
decoder schema are unchanged.
The Rust regression covers both requested shapes, standard/legacy encoding,
empty/pruned files, and a struct wrapper. The Scala regression confirms native
execution and Spark/Comet rejection on Spark 3.5.9 and 4.1.3, with standard
LIST controls still readable. Both reported regressions failed with the new
guard disabled. Final schema-adapter tests: 88 passed; full ParquetReadV1Suite:
78 passed on 4.1.3 and 76 on 3.5.9 (two expected widening cancellations). The
existing #6506 empty/pruned tests also pass.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
