andygrove commented on code in PR #6515:
URL: https://github.com/apache/datafusion-comet/pull/6515#discussion_r4162152851
##########
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:
Fixed in 3c252caccfa954300d404b495762d667377fa6e1. With row-filter pushdown
enabled, requested-column conversion checks retain the deferred error and raise
it on the first surviving data-page request, before row selection.
Footer/Bloom-filter reads and row-group/page-index pruning still run first.
DataFusion replaces its reader after Bloom-filter pruning, so the failure also
survives in the scan-scoped reader factory, with ObjectMeta checked before
reuse.
The regression asserts SchemaColumnConvertNotSupportedException in Spark and
Comet for id=[1,3], id=2, top-level/nested BINARY-to-int, with pushdown off/on.
Pruned row groups, fully page-pruned data, and unrequested mismatched columns
remain readable. The obsolete documented limitation was removed.
Local validation: 585 native tests passed, 5 ignored; final schema-adapter
tests 88 passed; full ParquetReadV1Suite 78 passed on Spark 4.1.3 and 76 on
3.5.9 (two expected cancellations). JNI build, strict Scala warnings, Clippy,
formatting and style checks passed. Upstream Spark SQL CI is pending on this
head; the existing run-spark-4.1-tests label should include it in the push run.
Iceberg JVM suites, other local Spark profiles and benchmarks were not rerun.
--
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]
