You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "peterxcli (via GitHub)" <gi...@apache.org> on 2026/09/11 19:11:53 UTC
[PR] feat: support direct Variant projection in native Parquet scans [datafusion-comet]
peterxcli opened a new pull request, #5868:
URL: https://github.com/apache/datafusion-comet/pull/5868
## Which issue does this PR close?
Closes #5551.
## Rationale for this change
Complete the ordinary-Parquet admission step in #5546 so Spark can project whole Variant values through a native scan.
## What changes are included in this PR?
Enable direct top-level Variant projection with `allowReadingShredded=true` and `pushVariantIntoScan=false`. Carry missing-column defaults as constant `[value, metadata]` structs while preserving their schema indexes, and retain Spark fallbacks for unsupported consumers and reader settings.
Reuse the Variant normalizer to handle Spark's empty object keys until Comet upgrades to an Arrow release containing apache/arrow-rs#10352. Document the supported surface and add removal TODOs for the compatibility paths, including #5477.
## How are these changes tested?
Focused Spark 4.0/4.1 tests cover value parity, defaults, Unicode field matching, vector layout, and fallback behavior. Native Variant tests and clippy pass, and Spark 3.5 compiles. The new projection suite runs in Linux and macOS CI.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support direct Variant projection in native Parquet scans [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on PR #5868:
URL: https://github.com/apache/datafusion-comet/pull/5868#issuecomment-5736899668
Merging, thanks @peterxcli !
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support direct Variant projection in native Parquet scans [datafusion-comet]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #5868:
URL: https://github.com/apache/datafusion-comet/pull/5868#issuecomment-5775210055
@sunchao could you help me to merge this again? thanks!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support direct Variant projection in native Parquet scans [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #5868:
URL: https://github.com/apache/datafusion-comet/pull/5868#discussion_r4019228209
##########
native/core/src/parquet/cast_column/variant.rs:
##########
@@ -57,22 +58,49 @@ pub(super) fn normalize_variant_array(
}
// VariantArray resolves metadata/value/typed_value by name, so the reader's child order is
- // irrelevant. Legacy Spark residuals must be put in Arrow order before the single upstream
- // unshred call; the whole output is then put back in the order expected by released Spark 4.
+ // irrelevant. Legacy Spark residuals must be put in Arrow order before unshredding;
+ // shredded output is then rebuilt with Spark's byte encoding.
let array = normalize_variant_storage(array)?;
let variant = VariantArray::try_new(array.as_ref())?;
- let prepared = prepare_variant_for_unshredding(&variant)?;
- let unshredded = unshred_variant(&prepared)?;
- let value = unshredded.value_column();
- let value = cast(value.as_ref(), &DataType::Binary)?;
- let metadata = cast(unshredded.metadata_column().as_ref(), &DataType::Binary)?;
- let value = reorder_variant_values(&value, &metadata, unshredded.inner().nulls())?;
-
- Ok(Arc::new(StructArray::try_new(
- fields.clone(),
- vec![value, metadata],
- unshredded.inner().nulls().cloned(),
- )?))
+ let normalize = |metadata: Option<&ArrayRef>| -> DataFusionResult<ArrayRef> {
+ let extended = extend_shredded_metadata(&variant, metadata)?;
+ let prepared = prepare_variant_for_unshredding(&variant, extended.as_ref().or(metadata))?;
+ let unshredded = unshred_variant(&prepared).map_err(|error| match error {
+ ArrowError::InvalidArgumentError(_) => {
+ DataFusionError::from(SparkError::MalformedVariant)
+ }
+ error => error.into(),
+ })?;
+ let (value, metadata) = if variant.typed_value_column().is_some() {
+ let value = cast(unshredded.value_column().as_ref(), &DataType::Binary)?;
+ let metadata = cast(unshredded.metadata_column().as_ref(), &DataType::Binary)?;
+ rebuild_spark_variant(&variant, &value, &metadata)?
Review Comment:
### Correctness
[P2] Could you update the two existing scan-test expectations alongside this reconstruction change, keeping their wide physical decimal fixtures? `variant_scan_uses_parquet_physical_types_instead_of_arrow_schema_hints` and `variant_scan_reads_wide_physical_decimal_as_decimal128` still compare against `Variant::Decimal16` for the unscaled value `123` at scale `2`. The Spark-compatible narrowing here correctly returns `Decimal4`, so both assertions fail in [the current native test job](https://github.com/apache/datafusion-comet/actions/runs/35008373235/job/104514932231). I verified that both the normalizer and test file are identical between this head and the executed merge. This is a test-expectation update, not a request to restore the wide encoding. Please rerun the native suite after updating it. The current fail-fast run leaves 1,096 tests unexecuted.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support direct Variant projection in native Parquet scans [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on PR #5868:
URL: https://github.com/apache/datafusion-comet/pull/5868#issuecomment-5724670547
cc @andygrove @viirya @comphead let me know if you want to take another pass before I merge this
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support direct Variant projection in native Parquet scans [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao merged PR #5868:
URL: https://github.com/apache/datafusion-comet/pull/5868
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support direct Variant projection in native Parquet scans [datafusion-comet]
Posted by "dwsmith1983 (via GitHub)" <gi...@apache.org>.
dwsmith1983 commented on PR #5868:
URL: https://github.com/apache/datafusion-comet/pull/5868#issuecomment-5739783944
Hi @andygrove @sunchao @peterxcli
The last three merge-queue runs for this PR fail the same test on Linux and macOS, native scan declines top-level fields that repeat a Parquet field id, with the plan still holding a CometNativeScan (for example https://github.com/apache/datafusion-comet/actions/runs/35418573600). It also failed that way in the groups that included #5874 behind this PR.
The cause looks like the isSchemaSupported hunk in CometScanRule.scala: the per-field forall over requiredSchema.fields replaces the typeChecker.isSchemaSupported(scanExec.requiredSchema, ...) call, and #6004 put the root-level duplicate-field-id check in exactly that override, so it no longer runs. Keeping the schema-level call and applying the Variant admission on top of it, or checking duplicateFieldIds(requiredSchema.fields) before the loop, should bring the test back.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support direct Variant projection in native Parquet scans [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on PR #5868:
URL: https://github.com/apache/datafusion-comet/pull/5868#issuecomment-5826008584
Merging now!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org