andygrove commented on code in PR #4187:
URL: https://github.com/apache/datafusion-comet/pull/4187#discussion_r3184444195
##########
native/spark-expr/src/math_funcs/wide_decimal_binary_expr.rs:
##########
@@ -557,4 +576,66 @@ mod tests {
let arr = result.as_primitive::<Decimal128Type>();
assert_eq!(arr.value(0), 20000); // 2.0000
}
+
+ /// Regression test for the TPC-DS q23 BroadcastHashJoin crash (issue
#1615).
+ ///
+ /// When both inputs are `ColumnarValue::Scalar`, `evaluate` must return a
+ /// `ColumnarValue::Scalar` -- not a length-1 `ColumnarValue::Array`.
Otherwise
+ /// downstream comparisons against full batches see two `Array` operands
with
+ /// mismatched lengths and arrow-ord's `compare_op` rejects them with
+ /// "Cannot compare arrays of different lengths, got N vs 1".
+ #[test]
+ fn test_scalar_scalar_returns_scalar() {
+ use datafusion::common::ScalarValue;
+ use datafusion::physical_expr::expressions::Literal;
+
+ // 0.95 * 100.00 -- the same Scalar x Scalar decimal multiply pattern
that
+ // appears in the q23 filter `0.95 * scalar_subquery > ssales`.
+ let left: Arc<dyn PhysicalExpr> =
+ Arc::new(Literal::new(ScalarValue::Decimal128(Some(95), 38, 2)));
+ let right: Arc<dyn PhysicalExpr> =
+ Arc::new(Literal::new(ScalarValue::Decimal128(Some(10000), 38,
2)));
+
+ let expr = WideDecimalBinaryExpr::new(
+ left,
+ right,
+ WideDecimalOp::Multiply,
+ 38,
+ 2,
+ EvalMode::Legacy,
+ );
+
+ // Empty schema -- both inputs are Literals so no columns are needed.
+ let batch = RecordBatch::new_empty(Arc::new(Schema::empty()));
+ match expr.evaluate(&batch).unwrap() {
+ ColumnarValue::Scalar(ScalarValue::Decimal128(Some(v), 38, 2)) => {
+ // 0.95 * 100.00 = 95.00 -> at scale 2, integer 9500
+ assert_eq!(v, 9500);
+ }
+ ColumnarValue::Scalar(other) => {
+ panic!("expected Decimal128(Some(_), 38, 2), got {other:?}");
+ }
+ ColumnarValue::Array(_) => {
+ panic!(
+ "Scalar x Scalar must return ColumnarValue::Scalar, not
Array. This is the q23 BHJ crash regression (issue #1615)."
+ );
Review Comment:
As @coderfender mentioned, this is unrelated to #1615, which was for an
Arrow C-Data offset-buffer bug. Also there is some weird spacing in this
message.
```suggestion
panic!(
"Scalar x Scalar must return ColumnarValue::Scalar, not
Array"
);
```
--
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]
