pedrumj2 opened a new pull request, #13503:
URL: https://github.com/apache/gluten/pull/13503

   ## What changes are proposed in this pull request?
   
   
[kBlackList](https://github.com/apache/gluten/blob/d0d5105e01bf9cc67be654b9dac5f3ce3a96254d/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc#L59)
 rejects `sequence` by name, so a projection such as `sequence(1, size(items))` 
falls back with the reason `Function is not supported: sequence`, even though 
Velox has had a Spark `sequence` for integer types since 
facebookincubator/velox#16339, which the [Velox revision this repo 
pins](https://github.com/apache/gluten/blob/d0d5105e01bf9cc67be654b9dac5f3ce3a96254d/ep/build-velox/src/get-velox.sh#L21)
 includes. With this change those projections stay in Velox.
   
   A zero step with equal start and stop returns `[start]` in 
[Spark](https://github.com/apache/spark/blob/v3.5.5/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala#L3479-L3483)
 but throws `step must not be zero` in 
[Velox](https://github.com/facebookincubator/velox/blob/75cce7758dc3e6c3bbab826bf2423f118695ad57/velox/functions/lib/Sequence.cpp#L214).
 
[SubstraitToVeloxPlanValidator](https://github.com/apache/gluten/blob/d0d5105e01bf9cc67be654b9dac5f3ce3a96254d/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc)
 therefore offloads only calls with no step or a non-zero literal step; a 
column step, a zero step, and date or timestamp arguments still fall back.
   
   ## How was this patch tested?
   
   ### Local run
   
   Clone the repository:
   
   ```bash
   git clone https://github.com/apache/gluten.git && cd gluten
   ```
   
   Build the Velox backend for Spark 3.5 in the CI image:
   
   ```bash
   docker run --rm -v "$PWD":/work/gluten -w /work/gluten 
apache/gluten:centos-9-jdk8 \
     ./dev/buildbundle-veloxbe.sh --run_setup_script=OFF --build_arrow=OFF 
--spark_version=3.5
   ```
   
   Write the queries. `range()` keeps the arguments non-literal, so Spark 
cannot constant-fold the calls away:
   
   ```bash
   cat > q.sql <<'SQL'
   CREATE OR REPLACE TEMPORARY VIEW t AS SELECT id AS k FROM range(5);
   EXPLAIN SELECT sequence(k, k + 3) AS up, sequence(k + 3, k, -2) AS down FROM 
t;
   SELECT sequence(k, k + 3) AS up, sequence(k + 3, k, -2) AS down FROM t;
   SQL
   ```
   
   Run them with Gluten enabled:
   
   ```bash
   docker run --rm -v "$PWD":/work/gluten -w /work/gluten 
apache/gluten:centos-9-jdk8 bash -c '
     JAR=$(ls package/target/gluten-velox-bundle-spark3.5_*.jar)
     /opt/shims/spark35/spark_home/bin/spark-sql --master "local[2]" \
       --conf spark.plugins=org.apache.gluten.GlutenPlugin \
       --conf spark.driver.extraClassPath="$JAR" \
       --conf spark.executor.extraClassPath="$JAR" \
       --conf spark.memory.offHeap.enabled=true \
       --conf spark.memory.offHeap.size=2g \
       --conf 
spark.shuffle.manager=org.apache.spark.shuffle.sort.ColumnarShuffleManager \
       -f q.sql'
   ```
   
   On `main` at 
[d0d5105e01](https://github.com/apache/gluten/commit/d0d5105e01bf9cc67be654b9dac5f3ce3a96254d),
 the projection that calls `sequence` falls back to vanilla Spark:
   
   ```
   …
   26/10/30 19:28:53 WARN GlutenFallbackReporter: Validation failed for plan: 
Project[QueryId=1], due to:
    - Native validation failed:
      |- Validation failed at file:SubstraitToVeloxPlanValidator.cc, line:228, 
function:validateScalarFunction, reason:Function is not supported: sequence
   == Physical Plan ==
   *(1) Project [sequence(id#11L, (id#11L + 3), None, Some(Etc/UTC)) AS up#2, 
sequence((id#11L + 3), id#11L, Some(-2), Some(Etc/UTC)) AS down#3]
   +- *(1) ColumnarToRow
      +- ColumnarRange 0, 5, 1, 2, 5, [id#11L]
   …
   [0,1,2,3]    [3,1]
   [1,2,3,4]    [4,2]
   [2,3,4,5]    [5,3]
   [3,4,5,6]    [6,4]
   [4,5,6,7]    [7,5]
   …
   ```
   
   Then check out this pull request's branch and repeat the build and run 
steps. The projection now runs in Velox and returns the same rows:
   
   ```
   …
   == Physical Plan ==
   VeloxColumnarToRow
   +- ^(1) ProjectExecTransformer [sequence(id#11L, (id#11L + 3), None, 
Some(Etc/UTC)) AS up#2, sequence((id#11L + 3), id#11L, Some(-2), Some(Etc/UTC)) 
AS down#3]
      +- ^(1) InputIteratorTransformer[id#11L]
         +- ArrowColumnarToVeloxColumnar
            +- OffloadArrowData
               +- ColumnarRange 0, 5, 1, 2, 5, [id#11L]
   …
   [0,1,2,3]    [3,1]
   [1,2,3,4]    [4,2]
   [2,3,4,5]    [5,3]
   [3,4,5,6]    [6,4]
   [4,5,6,7]    [7,5]
   …
   ```
   
   ### Automated tests
   
   Two tests added to 
[ScalarFunctionsValidateSuite](https://github.com/apache/gluten/blob/d0d5105e01bf9cc67be654b9dac5f3ce3a96254d/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala)
 check that integer sequences across all four integer types offload and match 
vanilla Spark, and that date arguments, a column step, and a zero step fall 
back.
   
   ```bash
   ./build/mvn -o -B test -Pbackends-velox -Pspark-3.5 -pl backends-velox 
-Dspotless.check.skip=true -Dscalastyle.skip=true -Dcheckstyle.skip=true 
-Dtest=none -DfailIfNoTests=false 
-DwildcardSuites=org.apache.gluten.functions.ScalarFunctionsValidateSuite
   ```
   
   ```
   …
   - sequence function optimized by Spark constant folding
   …
   - sequence with integer arguments
   …
   - sequence with date arguments or a non-literal step falls back
   …
   - map_from_arrays optimized by Spark constant folding
   …
   - map_from_arrays offloads to Velox under both mapKeyDedupPolicy values
   …
   Tests: succeeded 88, failed 0, canceled 0, ignored 0, pending 0
   All tests passed.
   ```
   
   ```bash
   ./build/mvn -o -B test -Pspark-ut -Pbackends-velox -Pspark-3.5 -pl 
gluten-ut/spark35 -Dspotless.check.skip=true -Dscalastyle.skip=true 
-Dcheckstyle.skip=true -Dtest=none -DfailIfNoTests=false 
-DwildcardSuites=org.apache.spark.sql.catalyst.expressions.GlutenCollectionExpressionsSuite,org.apache.spark.sql.GlutenDataFrameFunctionsSuite
   ```
   
   ```
   GlutenCollectionExpressionsSuite:
   …
   - Sequence of numbers
   …
   - Sequence of timestamps
   …
   - Sequence on DST boundaries
   …
   - Sequence of dates
   …
   - SPARK-37544: Time zone should not affect date sequence with month interval
   …
   - SPARK-35088: Accept ANSI intervals by the Sequence expression
   …
   - SPARK-36090: Support TimestampNTZType in expression Sequence
   …
   - Sequence with default step
   …
   GlutenDataFrameFunctionsSuite:
   …
   - sequence
   …
   Tests: succeeded 194, failed 0, canceled 0, ignored 7, pending 0
   All tests passed.
   ```
   
   ## Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Co-authored with Claude Code (claude-opus-5-5)
   
   <!--
   Thank you for submitting a pull request! Here are some tips:
   
   1. For first-time contributors, please read our contributing guide:
      https://github.com/apache/gluten/blob/main/CONTRIBUTING.md
   2. If necessary, create a GitHub issue for discussion beforehand to avoid 
duplicate work.
   3. If the PR is specific to a single backend, include [VL] or [CH] in the PR 
title to indicate the
      Velox or ClickHouse backend, respectively.
   4. If the PR is not ready for review, please mark it as a draft.
   -->
   


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

Reply via email to