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: [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]
