andygrove commented on code in PR #6515:
URL: https://github.com/apache/datafusion-comet/pull/6515#discussion_r4162472890
##########
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()),
+ DataType::Map(entries, _) => matches!(
+ entries.data_type(),
+ DataType::Struct(kv) if kv.iter().all(|f|
!is_complex(f.data_type()))
+ ),
+ _ => false,
+ };
+ if !is_unclipped {
+ return Err(parquet_schema_convert_err(
+ column,
+ physical_type,
+ target_type,
+ ));
+ }
+ } else if spark_has_updater(physical_type, target_type, options) {
+ return Ok(ConversionCheck::Accept);
+ }
+ Ok(ConversionCheck::RejectOnNonEmpty {
Review Comment:
The upstream Spark 4.1 SQL run caught an additional error-propagation issue
in this path: the two SPARK-25207 duplicate-field tests received a generic
SparkException because footer validation wrapped a structured Spark error
twice. Fixed in d9b754d7031f286b98c143baba4cc46bb2e0c369 by preserving the
original External error payload.
Added a native footer-error typing regression and a JVM duplicate-field
regression with row-filter pushdown both off and on. The JVM regression
reproduced the CI failure before rebuilding JNI and passes with the fix.
Validation: 586 native tests passed (5 ignored), full Spark 4.1
ParquetReadV1Suite 79 passed (1 existing ignored), full Spark 3.5
strict-warnings ParquetReadV1Suite 77 passed (2 expected widening
cancellations, 1 existing ignored), Clippy, formatting, and reactor lint all
passed.
The previous CI run passed all other jobs and 8 of 9 SQL shards.
Current-head CI is now running, including all Spark profiles and upstream Spark
4.1 SQL: https://github.com/apache/datafusion-comet/actions/runs/36957821361 .
No merge or queue action taken.
--
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]
