Skip to content

[VL] Fix CheckOverflowTransformer using wrong child type for cast decision#12261

Open
Xtpacz wants to merge 3 commits into
apache:mainfrom
Xtpacz:fix-decimal-pb
Open

[VL] Fix CheckOverflowTransformer using wrong child type for cast decision#12261
Xtpacz wants to merge 3 commits into
apache:mainfrom
Xtpacz:fix-decimal-pb

Conversation

@Xtpacz

@Xtpacz Xtpacz commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Fix: #12260

What changes are proposed in this pull request?

CheckOverflowTransformer reads original.child.dataType to decide whether to insert a cast. For BinaryArithmetic, Spark's .dataType returns left.dataType rather than the arithmetic result type. After child transformers apply rescale optimizations, the actual output type may differ from the Spark-declared type, and the cast is wrongly skipped.

The resulting substrait plan has decimal types that mismatch function signatures. Velox's SimpleFunction validation rejects it, and ColumnarPartialProjectRule falls the entire Project back to JVM. Result is correct (via fallback) but native acceleration is lost.

Reproducer

CREATE TABLE t1 (val BIGINT) USING parquet;
CREATE TABLE t2 (val BIGINT) USING parquet;
INSERT INTO t1 VALUES (200);
INSERT INTO t2 VALUES (100), (100), (100), (100), (100);

SELECT
    a.val,
    (a.val - COALESCE(SUM(b.val), 0) / 5.0)
        / (COALESCE(SUM(b.val), 0) / 5.0) AS growth_rate
FROM t1 a CROSS JOIN t2 b
GROUP BY a.val;

Root cause:


this read the Spark expression's declared type instead of the transformer's actual output type.

Fix

- original.child.dataType,
+ child.dataType,

How was this patch tested?

Before fix — Project falls back to JVM:

== Final Plan ==
* Project (17)                                         ← JVM, codegen id=3
+- VeloxColumnarToRow (16)                             ← extra C2R conversion
   +- ^ RegularHashAggregateExecTransformer (14)
      +- ^ VeloxBroadcastNestedLoopJoinExecTransformer (13)
         :- ^ InputIteratorTransformer (7)
         :  +- BroadcastQueryStage (5)
         :     +- ColumnarBroadcastExchange (4)
         :        +- RowToVeloxColumnar (3)
         :           +- * ColumnarToRow (2)
         :              +- BatchScan (1)
         +- ^ InputIteratorTransformer (12)
            +- RowToVeloxColumnar (10)
               +- * ColumnarToRow (9)
                  +- BatchScan (8)

After fix — Project runs natively in Velox:

== Final Plan ==
VeloxColumnarToRow (17)
+- ^ ProjectExecTransformer (15)                       ← native Velox Project
   +- ^ RegularHashAggregateExecTransformer (14)
      +- ^ VeloxBroadcastNestedLoopJoinExecTransformer (13)
         :- ^ InputIteratorTransformer (7)
         :  +- BroadcastQueryStage (5)
         :     +- ColumnarBroadcastExchange (4)
         :        +- RowToVeloxColumnar (3)
         :           +- * ColumnarToRow (2)
         :              +- BatchScan (1)
         +- ^ InputIteratorTransformer (12)
            +- RowToVeloxColumnar (10)
               +- * ColumnarToRow (9)
                  +- BatchScan (8)

Key differences:

  • Node (15) changes from Project (JVM, * = codegen) to ProjectExecTransformer (Velox native, ^ = transformer)
  • VeloxColumnarToRow moves from before Project (forced conversion to feed JVM) to after Project (deferred until output)
  • Aggregate→Project pipeline stays in Velox without breaking

@github-actions github-actions Bot added the CORE works for Gluten Core label Jun 8, 2026
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@Xtpacz

Xtpacz commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@zhouyuan @philo-he could you have a look at the PR?

@Xtpacz

Xtpacz commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@JkSelf @wForget could you have a look at the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CORE works for Gluten Core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[VL] Project with decimal arithmetic falls back to JVM when aggregate result is divided by integer-valued decimal literal

1 participant