sunchao commented on code in PR #5889:
URL: https://github.com/apache/datafusion-comet/pull/5889#discussion_r4332877116
##########
native/spark-expr/src/struct_funcs/get_struct_field.rs:
##########
@@ -99,9 +99,15 @@ impl PhysicalExpr for GetStructField {
self.ordinal,
)?))
}
- ColumnarValue::Scalar(ScalarValue::Struct(struct_array)) =>
Ok(ColumnarValue::Array(
- child_with_parent_nulls(&struct_array, self.ordinal)?,
- )),
+ ColumnarValue::Scalar(ScalarValue::Struct(struct_array)) => {
+ let child = child_with_parent_nulls(&struct_array,
self.ordinal)?;
+ // Match a struct column's batch-sized output so downstream
expressions,
+ // including IF's whole-batch branch shortcut, never receive a
scalar field.
+ Ok(ColumnarValue::Array(
+ ScalarValue::try_from_array(child.as_ref(), 0)?
+ .to_array_of_size(batch.num_rows())?,
Review Comment:
[P2] Avoid eagerly expanding variable-width scalar fields before their
consumers. With AQE disabled, subquery reuse enabled, two 256 KiB strings in
Parquet table `src`, and an 8,192-row outer batch, `SELECT id,
octet_length((SELECT max(s) FROM src)), octet_length((SELECT min(s) FROM src))
FROM outer_rows` produces merged struct subqueries. Spark returns `262144` for
both lengths on every row. This expansion instead attempts a 2,147,483,648-byte
`Utf8` value buffer and panics with `offset overflow` before `octet_length`
runs. Smaller inputs still incur unnecessary copies proportional to batch size,
even though the final result contains only integers. The base and release
retain Spark fallback for this struct shape. Please preserve scalar evaluation
for scalar-capable consumers or provide bounded expansion without duplicating
the full value, while retaining the fixes for array-dependent consumers.
Evidence: Reproduced at the reviewed SHA with `GetStructField` over
`ScalarValue::Struct` containing a 262,144-byte `Utf8`, feeding DataFusion’s
`OctetLengthFunc` on an 8,192-row batch. Evaluation panicked at
`arrow-buffer-59.3.0/src/buffer/offset.rs:167` with `offset overflow`; the
equivalent scalar consumer returned `Scalar(Int32(262144))`. Arrow checks the
offset before allocating the repeated buffer, so this reproduction did not
allocate 2 GiB. A separate 1,024-row test measured 33,554,432 value bytes from
a 32,768-byte scalar. Spark 3.5.9 executed the SQL reference successfully,
showed merged struct subqueries in its optimized plan, and returned 8,192 rows
with `(262144, 262144)`. Native SQL admission and the prior fallback were
verified from source.
--
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]
