From 0f82e51b6952c91181a48fb10d958f99bc35f892 Mon Sep 17 00:00:00 2001 From: Yuan Date: Mon, 24 Aug 2026 11:19:22 +0100 Subject: [PATCH 1/3] [VL] Support Spark ANSI behavior for elt through the function overlay Gluten has no per-function support for Spark's ANSI mode yet. Spark's elt raises an error on an out-of-range index when ANSI mode is on and returns NULL otherwise, and Velox has no elt at all, so the expression always falls back today. Implement elt in Gluten's function overlay instead of waiting for it to land in Velox. It is a vector function covering both elt(integer, varchar...) and elt(integer, varbinary...): a NULL index or a NULL selected input gives NULL, NULLs in inputs that are not selected are ignored, and an index outside [1, number of inputs] gives NULL with ANSI mode off and a user error with it on. ANSI mode is read from SparkQueryConfig::ansiEnabled(), which Gluten already populates from the session's spark.sql.ansi.enabled, the same channel Velox's own sparksql functions use. A vector function is used rather than a simple function because Velox's SimpleFunctionAdapter has no VectorReader> constructor on the initialize() path, so a variadic simple function cannot read the query config. Rows are grouped by the input they select and each input is then copied in one pass, which lets the result share the input's string buffers. Spark captures the ANSI decision in Elt.failOnError at analysis time while Velox derives it from the session config, so fall back when the two disagree, which can happen if spark.sql.ansi.enabled changes between analysis and execution. Note that docs/velox-backend-scalar-function-support.md is generated by tools/scripts/gen-function-support-docs.py from a test run and still lists elt as unsupported; it needs a regeneration. Co-Authored-By: Claude Opus 5 --- .../velox/VeloxSparkPlanExecApi.scala | 17 +++ .../expression/ExpressionRestrictions.scala | 12 ++ .../ScalarFunctionsValidateSuite.scala | 20 +++ .../functions/StringAnsiValidateSuite.scala | 56 +++++++++ cpp/velox/CMakeLists.txt | 1 + cpp/velox/operators/functions/overlay/Elt.cc | 119 ++++++++++++++++++ cpp/velox/operators/functions/overlay/Elt.h | 49 ++++++++ .../operators/functions/overlay/README.md | 3 +- .../overlay/RegisterFunctionOverlay.cc | 9 ++ cpp/velox/tests/SparkFunctionTest.cc | 76 +++++++++++ .../velox-function-development-guide.md | 8 +- .../gluten/backendsapi/SparkPlanExecApi.scala | 7 ++ .../expression/ExpressionConverter.scala | 6 + 13 files changed, 381 insertions(+), 2 deletions(-) create mode 100644 backends-velox/src/test/scala/org/apache/gluten/functions/StringAnsiValidateSuite.scala create mode 100644 cpp/velox/operators/functions/overlay/Elt.cc create mode 100644 cpp/velox/operators/functions/overlay/Elt.h diff --git a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala index d45b7a3f200..af00985b884 100644 --- a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala +++ b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala @@ -1250,6 +1250,23 @@ class VeloxSparkPlanExecApi extends SparkPlanExecApi with Logging { GenericExpressionTransformer(substraitExprName, child, expr) } + override def genEltTransformer( + substraitExprName: String, + children: Seq[ExpressionTransformer], + expr: Elt): ExpressionTransformer = { + // Velox's elt derives whether an out-of-range index raises an error from the session's + // 'spark.sql.ansi.enabled', while Spark captures it in Elt.failOnError at analysis time. + // The two normally agree; fall back when they don't, so the ANSI behavior never diverges. + if (expr.failOnError != SQLConf.get.ansiEnabled) { + GlutenExceptionUtil + .throwsNotFullySupported( + ExpressionNames.ELT, + EltRestrictions.NOT_SUPPORT_FAIL_ON_ERROR_MISMATCH + ) + } + GenericExpressionTransformer(substraitExprName, children, expr) + } + override def genBase64StaticInvokeTransformer( substraitExprName: String, child: ExpressionTransformer, diff --git a/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala b/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala index b6bfd4119be..4cf3f7d6378 100644 --- a/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala +++ b/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala @@ -83,6 +83,17 @@ object Unbase64Restrictions extends ExpressionRestrictions { override val restrictionMessages: Array[String] = Array(NOT_SUPPORT_FAIL_ON_ERROR) } +object EltRestrictions extends ExpressionRestrictions { + val NOT_SUPPORT_FAIL_ON_ERROR_MISMATCH: String = + s"${ExpressionNames.ELT} whose failOnError disagrees with the session's " + + s"'${SQLConf.ANSI_ENABLED.key}' is not supported, since Velox derives the ANSI " + + s"behavior of elt from the session config" + + override val functionName: String = ExpressionNames.ELT + + override val restrictionMessages: Array[String] = Array(NOT_SUPPORT_FAIL_ON_ERROR_MISMATCH) +} + object Base64Restrictions extends ExpressionRestrictions { val NOT_SUPPORT_DISABLE_CHUNK_BASE64_STRING: String = s"${ExpressionNames.BASE64} with chunkBase64String disabled is not supported" @@ -125,6 +136,7 @@ object ExpressionRestrictions { ToJsonRestrictions, Unbase64Restrictions, Base64Restrictions, + EltRestrictions, FormatNumberRestrictions ) } diff --git a/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala b/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala index 557b86a17dd..19f60238a6b 100644 --- a/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala +++ b/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala @@ -177,6 +177,26 @@ class ScalarFunctionsValidateSuite extends FunctionsValidateSuite { } } + test("elt") { + withSQLConf(SQLConf.ANSI_ENABLED.key -> "false") { + // int_field1 is 1, 2, 3, so every input gets selected by some row. + runQueryAndCompare("SELECT elt(int_field1, string_field1, 'b', 'c') FROM datatab") { + checkGlutenPlan[ProjectExecTransformer] + } + // A NULL index, an out-of-range index and a NULL selected input all give NULL + // with ANSI mode off. + runQueryAndCompare( + "SELECT elt(NULL, 'a', 'b'), elt(int_field1 + 3, 'a', 'b'), " + + "elt(1, string_field1, 'b') FROM datatab") { + checkGlutenPlan[ProjectExecTransformer] + } + runQueryAndCompare( + "SELECT elt(int_field1, cast(string_field1 as binary), cast('b' as binary)) FROM datatab") { + checkGlutenPlan[ProjectExecTransformer] + } + } + } + test("shiftright") { runQueryAndCompare("SELECT shiftright(int_field1, 1) from datatab limit 1") { checkGlutenPlan[ProjectExecTransformer] diff --git a/backends-velox/src/test/scala/org/apache/gluten/functions/StringAnsiValidateSuite.scala b/backends-velox/src/test/scala/org/apache/gluten/functions/StringAnsiValidateSuite.scala new file mode 100644 index 00000000000..0c14f6dbad0 --- /dev/null +++ b/backends-velox/src/test/scala/org/apache/gluten/functions/StringAnsiValidateSuite.scala @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.gluten.functions + +import org.apache.gluten.config.GlutenConfig +import org.apache.gluten.execution.ProjectExecTransformer + +import org.apache.spark.SparkConf +import org.apache.spark.SparkException +import org.apache.spark.sql.internal.SQLConf + +class StringAnsiValidateSuite extends FunctionsValidateSuite { + + disableFallbackCheck + + override protected def sparkConf: SparkConf = { + super.sparkConf + .set(GlutenConfig.GLUTEN_ANSI_FALLBACK_ENABLED.key, "false") + .set(SQLConf.ANSI_ENABLED.key, "true") + } + + test("elt") { + // int_field1 is 1, 2, 3, so every index is within range here. + runQueryAndCompare("SELECT elt(int_field1, 'a', 'b', 'c') FROM datatab") { + checkGlutenPlan[ProjectExecTransformer] + } + + // A NULL index gives NULL rather than an error, and a NULL selected input stays NULL. + runQueryAndCompare("SELECT elt(NULL, 'a', 'b'), elt(1, string_field1, 'b') FROM datatab") { + checkGlutenPlan[ProjectExecTransformer] + } + + // An out-of-range index raises an error in ANSI mode. int_field1 - 1 is 0 for the + // first row, and int_field1 + 3 is beyond the number of inputs for every row. + intercept[SparkException] { + sql("SELECT elt(int_field1 - 1, 'a', 'b', 'c') FROM datatab").collect() + } + intercept[SparkException] { + sql("SELECT elt(int_field1 + 3, 'a', 'b', 'c') FROM datatab").collect() + } + } +} diff --git a/cpp/velox/CMakeLists.txt b/cpp/velox/CMakeLists.txt index c881ea8d57f..2ad8680815b 100644 --- a/cpp/velox/CMakeLists.txt +++ b/cpp/velox/CMakeLists.txt @@ -176,6 +176,7 @@ set(VELOX_SRCS memory/VeloxMemoryManager.cc operators/functions/RegistrationAllFunctions.cc operators/functions/delta/DeltaBitmapAggregator.cc + operators/functions/overlay/Elt.cc operators/functions/overlay/RegisterFunctionOverlay.cc operators/functions/RowConstructorWithNull.cc operators/functions/SparkExprToSubfieldFilterParser.cc diff --git a/cpp/velox/operators/functions/overlay/Elt.cc b/cpp/velox/operators/functions/overlay/Elt.cc new file mode 100644 index 00000000000..db6b69b427c --- /dev/null +++ b/cpp/velox/operators/functions/overlay/Elt.cc @@ -0,0 +1,119 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include "operators/functions/overlay/Elt.h" + +#include "velox/common/base/Status.h" +#include "velox/common/base/VeloxException.h" +#include "velox/expression/EvalCtx.h" +#include "velox/functions/sparksql/SparkQueryConfig.h" + +using namespace facebook::velox; + +namespace gluten { +namespace { + +/// Spark's elt(n, input1, input2, ...) returns the n-th input, 1-based. +/// +/// The inputs are all VARCHAR or all VARBINARY, so a single implementation +/// covers both: the result is copied verbatim out of the selected input, which +/// also lets the copy share the input's string buffers. +class EltFunction final : public exec::VectorFunction { + public: + explicit EltFunction(bool ansiEnabled) : ansiEnabled_{ansiEnabled} {} + + void apply( + const SelectivityVector& rows, + std::vector& args, + const TypePtr& outputType, + exec::EvalCtx& context, + VectorPtr& result) const override { + // args[0] holds the 1-based index, args[1:] hold the candidate inputs. + VELOX_CHECK_GE(args.size(), 2, "elt expects an index and at least one input."); + const auto numInputs = static_cast(args.size()) - 1; + + exec::LocalDecodedVector indexHolder(context, *args[0], rows); + const auto* indexes = indexHolder.get(); + + context.ensureWritable(rows, outputType, result); + // Rows that end up selecting no input keep this NULL: a NULL index, or an + // out-of-range index with ANSI mode off. + rows.applyToSelected([&](vector_size_t row) { result->setNull(row, true); }); + + // Group the rows by the input they select, so that each input is copied in + // a single pass. Most rows share the same index in practice, e.g. when the + // index is a constant. + std::vector> inputRows(numInputs); + rows.applyToSelected([&](vector_size_t row) { + if (indexes->isNullAt(row)) { + return; + } + const auto index = indexes->valueAt(row); + if (index < 1 || index > numInputs) { + if (ansiEnabled_) { + context.setStatus(row, invalidIndexStatus(index, numInputs)); + } + return; + } + auto& selected = inputRows[index - 1]; + if (selected == nullptr) { + selected = std::make_unique(rows.end(), false); + } + selected->setValid(row, true); + }); + + for (int32_t i = 0; i < numInputs; ++i) { + if (inputRows[i] != nullptr) { + inputRows[i]->updateBounds(); + // Copies the values and the nulls, so a NULL in the selected input + // becomes a NULL result. + result->copy(args[i + 1].get(), *inputRows[i], nullptr); + } + } + } + + private: + static Status invalidIndexStatus(int32_t index, int32_t numInputs) { + if (threadSkipErrorDetails()) { + return Status::UserError(); + } + return Status::UserError("The index is out of bounds for elt. index: {}, number of inputs: {}", index, numInputs); + } + + const bool ansiEnabled_; +}; + +} // namespace + +std::vector eltSignatures() { + return { + exec::FunctionSignatureBuilder().returnType("varchar").argumentType("integer").variableArity("varchar").build(), + exec::FunctionSignatureBuilder() + .returnType("varbinary") + .argumentType("integer") + .variableArity("varbinary") + .build(), + }; +} + +std::shared_ptr makeElt( + const std::string& /*name*/, + const std::vector& /*inputArgs*/, + const core::QueryConfig& config) { + return std::make_shared(functions::sparksql::SparkQueryConfig{config}.ansiEnabled()); +} + +} // namespace gluten diff --git a/cpp/velox/operators/functions/overlay/Elt.h b/cpp/velox/operators/functions/overlay/Elt.h new file mode 100644 index 00000000000..1beb53627f4 --- /dev/null +++ b/cpp/velox/operators/functions/overlay/Elt.h @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +#include "velox/expression/VectorFunction.h" + +namespace gluten { + +/// Signatures of Spark's elt: elt(n, input1, input2, ...) where all the inputs +/// are VARCHAR, or all of them are VARBINARY. +std::vector eltSignatures(); + +/// Creates Spark's elt function, which returns the n-th input, 1-based. +/// +/// Returns NULL when 'n' is NULL or when the selected input is NULL. When 'n' +/// is out of the range [1, number of inputs], the result follows Spark's ANSI +/// rule: NULL with ANSI mode off, a user error with ANSI mode on. ANSI mode is +/// read from the query config, so it is fixed for the lifetime of the returned +/// function instance. +/// +/// Unlike Spark, which only evaluates the selected input, all the inputs are +/// evaluated, so an error raised by an input that 'n' does not select still +/// surfaces. This is the usual eager-evaluation difference of a Velox function +/// and not specific to elt. +std::shared_ptr makeElt( + const std::string& name, + const std::vector& inputArgs, + const facebook::velox::core::QueryConfig& config); + +} // namespace gluten diff --git a/cpp/velox/operators/functions/overlay/README.md b/cpp/velox/operators/functions/overlay/README.md index c265c22158d..8d8beef088f 100644 --- a/cpp/velox/operators/functions/overlay/README.md +++ b/cpp/velox/operators/functions/overlay/README.md @@ -24,7 +24,8 @@ the overlay takes precedence over the Velox implementation. 1. Implement the function in a header/source file in this directory, following Velox's function authoring APIs (simple function, vector function, aggregate, - or window function). See `Round.h` for a simple-function example and + or window function). See `Round.h` for a simple-function override and + `Elt.h`/`Elt.cc` for a vector function that is missing in Velox, plus the [Velox scalar functions guide](https://github.com/facebookincubator/velox/blob/main/velox/docs/develop/scalar-functions.rst). 2. Register it in `RegisterFunctionOverlay.cc` inside `registerFunctionOverlay()`. Use the same name Gluten's Substrait plan diff --git a/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc b/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc index d16e7636bdb..99415c2e4f1 100644 --- a/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc +++ b/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc @@ -16,6 +16,7 @@ */ #include "operators/functions/overlay/RegisterFunctionOverlay.h" +#include "operators/functions/overlay/Elt.h" #include "operators/functions/overlay/Round.h" #include "velox/functions/lib/RegistrationHelpers.h" @@ -36,10 +37,18 @@ void registerRoundFunction() { velox::registerFunction({"round"}); } +// Velox has no elt yet. It is registered here so Gluten can offload it, and it +// honors Spark's ANSI rule for an out-of-range index. +void registerEltFunction() { + velox::exec::registerStatefulVectorFunction( + "elt", eltSignatures(), makeElt, velox::exec::VectorFunctionMetadataBuilder().defaultNullBehavior(false).build()); +} + } // namespace void registerFunctionOverlay() { registerRoundFunction(); + registerEltFunction(); } } // namespace gluten diff --git a/cpp/velox/tests/SparkFunctionTest.cc b/cpp/velox/tests/SparkFunctionTest.cc index ceb979a5ede..6ebe4a57fac 100644 --- a/cpp/velox/tests/SparkFunctionTest.cc +++ b/cpp/velox/tests/SparkFunctionTest.cc @@ -144,3 +144,79 @@ TEST_F(SparkFunctionTest, expressionLevelLegacyCastIgnoresSessionAnsiOn) { facebook::velox::test::assertEqualVectors(makeFlatVector({-121}), evaluate(legacyCast, input)); } + +TEST_F(SparkFunctionTest, elt) { + // The index picks a different input per row. A NULL in an input that is not + // picked does not affect the result, while a NULL in the picked one does. + auto index = makeFlatVector({1, 2, 3, 3}); + auto first = makeNullableFlatVector({"a0", std::nullopt, "a2", "a3"}); + auto second = makeNullableFlatVector({std::nullopt, "b1", "b2", "b3"}); + auto third = makeNullableFlatVector({"c0", "c1", "c2", std::nullopt}); + + facebook::velox::test::assertEqualVectors( + makeNullableFlatVector({"a0", "b1", "c2", std::nullopt}), + evaluate("elt(c0, c1, c2, c3)", makeRowVector({index, first, second, third}))); +} + +TEST_F(SparkFunctionTest, eltNullIndex) { + // A NULL index returns NULL, with ANSI mode on as well. + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + auto index = makeNullableFlatVector({std::nullopt, 2}); + auto first = makeFlatVector({"a0", "a1"}); + auto second = makeFlatVector({"b0", "b1"}); + + facebook::velox::test::assertEqualVectors( + makeNullableFlatVector({std::nullopt, "b1"}), + evaluate("elt(c0, c1, c2)", makeRowVector({index, first, second}))); +} + +TEST_F(SparkFunctionTest, eltIndexOutOfRangeAnsiOff) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "false"}}); + auto index = makeFlatVector({0, -1, 3, 2}); + auto first = makeFlatVector({"a0", "a1", "a2", "a3"}); + auto second = makeFlatVector({"b0", "b1", "b2", "b3"}); + + facebook::velox::test::assertEqualVectors( + makeNullableFlatVector({std::nullopt, std::nullopt, std::nullopt, "b3"}), + evaluate("elt(c0, c1, c2)", makeRowVector({index, first, second}))); +} + +TEST_F(SparkFunctionTest, eltIndexOutOfRangeAnsiOn) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + auto first = makeFlatVector({"a0", "a1"}); + auto second = makeFlatVector({"b0", "b1"}); + + auto evaluateWithIndex = [&](const std::vector& indexes) { + return evaluate("elt(c0, c1, c2)", makeRowVector({makeFlatVector(indexes), first, second})); + }; + + VELOX_ASSERT_THROW(evaluateWithIndex({1, 0}), "The index is out of bounds for elt. index: 0, number of inputs: 2"); + VELOX_ASSERT_THROW(evaluateWithIndex({-1, 1}), "The index is out of bounds for elt. index: -1, number of inputs: 2"); + VELOX_ASSERT_THROW(evaluateWithIndex({1, 3}), "The index is out of bounds for elt. index: 3, number of inputs: 2"); + + // In-range indexes are unaffected by ANSI mode. + facebook::velox::test::assertEqualVectors(makeFlatVector({"a0", "b1"}), evaluateWithIndex({1, 2})); +} + +TEST_F(SparkFunctionTest, eltVarbinary) { + auto index = makeFlatVector({2, 1}); + auto first = makeNullableFlatVector({"a0", "a1"}, VARBINARY()); + auto second = makeNullableFlatVector({"b0", std::nullopt}, VARBINARY()); + + auto result = evaluate("elt(c0, c1, c2)", makeRowVector({index, first, second})); + ASSERT_EQ(result->type()->kind(), TypeKind::VARBINARY); + facebook::velox::test::assertEqualVectors(makeNullableFlatVector({"b0", "a1"}, VARBINARY()), result); +} + +TEST_F(SparkFunctionTest, eltConstantIndexOverDictionaryInput) { + // A constant index selects the same input for all rows, and the selected + // input may carry an encoding. + auto index = makeConstant(2, 3); + auto first = makeFlatVector({"a0", "a1", "a2"}); + auto second = makeFlatVector({"b0", "b1", "b2"}); + auto dictionary = BaseVector::wrapInDictionary(nullptr, makeIndices({2, 0, 1}), 3, second); + + facebook::velox::test::assertEqualVectors( + makeFlatVector({"b2", "b0", "b1"}), + evaluate("elt(c0, c1, c2)", makeRowVector({index, first, dictionary}))); +} diff --git a/docs/developers/velox-function-development-guide.md b/docs/developers/velox-function-development-guide.md index a73aefe64d4..e4a9e45dd28 100644 --- a/docs/developers/velox-function-development-guide.md +++ b/docs/developers/velox-function-development-guide.md @@ -50,10 +50,16 @@ takes precedence over the Velox implementation. Use the overlay when: * A Spark function is missing in Velox. Implement it in the overlay first so Gluten can offload it immediately, then upstream it - to Velox at your own pace. + to Velox at your own pace. The `elt` function (`overlay/Elt.h`, `overlay/Elt.cc`) is an example. * A Velox `sparksql` function has a semantic gap with Spark. Put the corrected implementation in the overlay to override it while the fix is pending upstream. The `round` function (`overlay/Round.h`) is an example of such an override. +An overlay function follows Spark's ANSI rule the same way Velox's `sparksql` functions do: read +`SparkQueryConfig::ansiEnabled()` from the query config, which Gluten populates from the session's +`spark.sql.ansi.enabled`. Returning NULL on invalid input with ANSI mode off and raising a user error with it on is what +`elt` does for an out-of-range index. Note that Gluten still falls back on ANSI mode as a whole unless +`spark.gluten.sql.ansiFallback.enabled` is set to `false`. + To add a function: 1. Implement it in a file under `cpp/velox/operators/functions/overlay/`, using Velox's function authoring APIs (simple function, vector function, aggregate, or window function). diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala index 4ca08a5ad62..48f1b9f19de 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala @@ -203,6 +203,13 @@ trait SparkPlanExecApi { GenericExpressionTransformer(substraitExprName, child, expr) } + def genEltTransformer( + substraitExprName: String, + children: Seq[ExpressionTransformer], + expr: Elt): ExpressionTransformer = { + GenericExpressionTransformer(substraitExprName, children, expr) + } + def genBase64StaticInvokeTransformer( substraitExprName: String, child: ExpressionTransformer, diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala index b70bbc87994..361cf67df1b 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala @@ -926,6 +926,12 @@ object ExpressionConverter extends SQLConfHelper with Logging { replaceWithExpressionTransformer0(u.child, attributeSeq, expressionsMap), u ) + case e: Elt => + BackendsApiManager.getSparkPlanExecApiInstance.genEltTransformer( + substraitExprName, + e.children.map(replaceWithExpressionTransformer0(_, attributeSeq, expressionsMap)), + e + ) case ce if BackendsApiManager.getSparkPlanExecApiInstance.expressionFlattenSupported(ce) => replaceFlattenedExpressionWithExpressionTransformer( substraitExprName, From 1787d5a8737012dc3510926063fce94d44a2092c Mon Sep 17 00:00:00 2001 From: Yuan Date: Mon, 24 Aug 2026 11:32:18 +0100 Subject: [PATCH 2/3] [VL] Support Spark ANSI behavior for conv through the function overlay Velox's conv always lets the base conversion overflow, which its own comment calls "consistent with Spark". That is only true with ANSI mode off: Spark's NumberConverter.encode() saturates to 2^64 - 1 in that case, but raises overflowInConvError when ANSI mode is on. conv is already reported as fully supported, so an ANSI query silently gets the saturated value today. Add an overlay override that keeps delegating the conversion to Velox's conv and only adds the missing error. Before delegating, and only when ANSI mode is on, it parses the digits the same way Velox does and raises a user error when they do not fit in an unsigned 64-bit integer. That is exactly Spark's overflow condition: the two checks in NumberConverter.encode() together detect that accumulating the next digit would pass 2^64 - 1, which is what std::from_chars reports as result_out_of_range. Delegating rather than copying keeps the conversion logic, including its handling of negative inputs and negative target bases, in one place, so the overlay does not have to be kept in sync with fixes to Velox's conv. The sign is applied after the digits are accumulated, so inputs like conv('-1', 10, 16) still wrap around instead of raising an error, and an invalid base or an empty input still gives NULL in ANSI mode. As for elt, fall back when Conv.ansiEnabled, captured at analysis time, disagrees with the session's spark.sql.ansi.enabled that Velox reads. Note that Velox's conv only skips leading spaces while Spark trims the input first, so an input led by another whitespace character, such as a tab, is parsed as 0 by Velox. That is a pre-existing difference unrelated to ANSI mode and is left alone here. Co-Authored-By: Claude Opus 5 --- .../velox/VeloxSparkPlanExecApi.scala | 17 +++ .../expression/ExpressionRestrictions.scala | 12 +++ .../MathFunctionsValidateSuite.scala | 45 ++++++++ cpp/velox/operators/functions/overlay/Conv.h | 102 ++++++++++++++++++ .../operators/functions/overlay/README.md | 5 +- .../overlay/RegisterFunctionOverlay.cc | 8 ++ cpp/velox/tests/SparkFunctionTest.cc | 55 ++++++++++ .../velox-function-development-guide.md | 4 +- .../gluten/backendsapi/SparkPlanExecApi.scala | 7 ++ .../expression/ExpressionConverter.scala | 6 ++ 10 files changed, 258 insertions(+), 3 deletions(-) create mode 100644 cpp/velox/operators/functions/overlay/Conv.h diff --git a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala index af00985b884..99d605a82c5 100644 --- a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala +++ b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala @@ -1267,6 +1267,23 @@ class VeloxSparkPlanExecApi extends SparkPlanExecApi with Logging { GenericExpressionTransformer(substraitExprName, children, expr) } + override def genConvTransformer( + substraitExprName: String, + children: Seq[ExpressionTransformer], + expr: Conv): ExpressionTransformer = { + // Velox derives whether an overflow raises an error from the session's + // 'spark.sql.ansi.enabled', while Spark captures it in Conv.ansiEnabled at analysis time. + // The two normally agree; fall back when they don't, so the ANSI behavior never diverges. + if (expr.ansiEnabled != SQLConf.get.ansiEnabled) { + GlutenExceptionUtil + .throwsNotFullySupported( + ExpressionNames.CONV, + ConvRestrictions.NOT_SUPPORT_ANSI_ENABLED_MISMATCH + ) + } + GenericExpressionTransformer(substraitExprName, children, expr) + } + override def genBase64StaticInvokeTransformer( substraitExprName: String, child: ExpressionTransformer, diff --git a/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala b/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala index 4cf3f7d6378..d9f0d180aa8 100644 --- a/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala +++ b/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala @@ -94,6 +94,17 @@ object EltRestrictions extends ExpressionRestrictions { override val restrictionMessages: Array[String] = Array(NOT_SUPPORT_FAIL_ON_ERROR_MISMATCH) } +object ConvRestrictions extends ExpressionRestrictions { + val NOT_SUPPORT_ANSI_ENABLED_MISMATCH: String = + s"${ExpressionNames.CONV} whose ansiEnabled disagrees with the session's " + + s"'${SQLConf.ANSI_ENABLED.key}' is not supported, since Velox derives the ANSI " + + s"behavior of conv from the session config" + + override val functionName: String = ExpressionNames.CONV + + override val restrictionMessages: Array[String] = Array(NOT_SUPPORT_ANSI_ENABLED_MISMATCH) +} + object Base64Restrictions extends ExpressionRestrictions { val NOT_SUPPORT_DISABLE_CHUNK_BASE64_STRING: String = s"${ExpressionNames.BASE64} with chunkBase64String disabled is not supported" @@ -137,6 +148,7 @@ object ExpressionRestrictions { Unbase64Restrictions, Base64Restrictions, EltRestrictions, + ConvRestrictions, FormatNumberRestrictions ) } diff --git a/backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala b/backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala index 81a9ad5cdba..4e49e29057f 100644 --- a/backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala +++ b/backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala @@ -20,6 +20,7 @@ import org.apache.gluten.config.GlutenConfig import org.apache.gluten.execution.{BatchScanExecTransformer, ProjectExecTransformer} import org.apache.spark.SparkConf +import org.apache.spark.SparkException import org.apache.spark.sql.Row import org.apache.spark.sql.internal.SQLConf @@ -64,6 +65,34 @@ class MathFunctionsValidateSuiteAnsiOn extends FunctionsValidateSuite { checkGlutenPlan[ProjectExecTransformer] } } + + test("conv") { + runQueryAndCompare( + "select conv(cast(l_orderkey as string), 10, 16), conv('big', 36, 16) from lineitem") { + checkGlutenPlan[ProjectExecTransformer] + } + + // 2^64 - 1 is the largest input that does not overflow, and the sign is applied after + // the digits are accumulated, so neither of these raises an error. + runQueryAndCompare( + "select conv('18446744073709551615', 10, 10), conv('-1', 10, 16) from lineitem") { + checkGlutenPlan[ProjectExecTransformer] + } + + // An out-of-range base gives NULL rather than an error. + runQueryAndCompare("select conv('15', 1, 10), conv('15', 10, 37) from lineitem") { + checkGlutenPlan[ProjectExecTransformer] + } + + // Digits that do not fit in an unsigned 64-bit integer overflow, which raises an + // error in ANSI mode instead of saturating. l_orderkey is at least 1, so the + // concatenated input always has more than 16 hexadecimal digits. + intercept[SparkException] { + sql( + "select conv(concat(cast(l_orderkey as string), '0000000000000000'), 16, 10)" + + " from lineitem").collect() + } + } } class MathFunctionsValidateSuite extends FunctionsValidateSuite { @@ -343,6 +372,22 @@ class MathFunctionsValidateSuite extends FunctionsValidateSuite { compareResultsAgainstVanillaSpark("select round(44, -1)", true, { _ => }) } + test("conv") { + withSQLConf(SQLConf.ANSI_ENABLED.key -> "false") { + runQueryAndCompare( + "SELECT conv(cast(l_orderkey as string), 10, 16), conv('big', 36, 16) from lineitem") { + checkGlutenPlan[ProjectExecTransformer] + } + + // Digits that do not fit in an unsigned 64-bit integer saturate with ANSI mode off. + runQueryAndCompare( + "SELECT conv(concat(cast(l_orderkey as string), '0000000000000000'), 16, 10)," + + " conv('9223372036854775807', 36, 16) from lineitem") { + checkGlutenPlan[ProjectExecTransformer] + } + } + } + test("shiftleft") { runQueryAndCompare("SELECT shiftleft(int_field1, 1) from datatab limit 1") { checkGlutenPlan[ProjectExecTransformer] diff --git a/cpp/velox/operators/functions/overlay/Conv.h b/cpp/velox/operators/functions/overlay/Conv.h new file mode 100644 index 00000000000..bf88e2f3eca --- /dev/null +++ b/cpp/velox/operators/functions/overlay/Conv.h @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include +#include +#include +#include + +#include "velox/common/base/Exceptions.h" +#include "velox/functions/sparksql/SparkQueryConfig.h" +#include "velox/functions/sparksql/String.h" + +namespace gluten { + +/// conv(num, fromBase, toBase) -> varchar +/// +/// Overrides Velox's conv, which always lets the conversion overflow. Spark +/// only does that with ANSI mode off; with ANSI mode on, an input whose digits +/// do not fit in an unsigned 64-bit integer raises an error instead of being +/// saturated. Everything else, including the conversion itself, is delegated to +/// Velox's implementation. +template +struct ConvFunction { + VELOX_DEFINE_FUNCTION_TYPES(T); + + // ASCII input always produces ASCII result. + static constexpr bool is_default_ascii_behavior = true; + + void initialize( + const std::vector& /*inputTypes*/, + const facebook::velox::core::QueryConfig& config, + const arg_type* /*num*/, + const int32_t* /*fromBase*/, + const int32_t* /*toBase*/) { + ansiEnabled_ = facebook::velox::functions::sparksql::SparkQueryConfig{config}.ansiEnabled(); + } + + bool call( + out_type& result, + const arg_type& num, + int32_t fromBase, + int32_t toBase) { + if (FOLLY_UNLIKELY(ansiEnabled_) && overflows(num, fromBase, toBase)) { + // Same wording as Spark's QueryExecutionErrors.overflowInConvError. The + // simple function framework turns this into a per-row error. + VELOX_USER_FAIL("Overflow in function conv()"); + } + return delegate_.call(result, num, fromBase, toBase); + } + + private: + using VeloxConvFunction = facebook::velox::functions::sparksql::ConvFunction; + + /// Returns true when the digits of 'num' do not fit in an unsigned 64-bit + /// integer. That is exactly when Spark's NumberConverter.encode() reports an + /// overflow: its two checks together detect that accumulating the next digit + /// would pass 2^64 - 1. Locates and parses the digits the same way Velox's + /// conv does, so the two agree on where the digits end. + static bool overflows(const arg_type& num, int32_t fromBase, int32_t toBase) { + if (!VeloxConvFunction::checkInput(num, fromBase, toBase)) { + // An empty input or an out-of-range base gives NULL, in ANSI mode too. + return false; + } + auto position = static_cast(VeloxConvFunction::skipLeadingSpaces(num)); + if (position == num.size()) { + // All spaces. + return false; + } + // Skips the negative symbol, std::from_chars does not accept one for an + // unsigned type. Spark applies the sign after the digits are accumulated, + // so it does not affect whether the digits overflow. + if (num.data()[position] == '-') { + ++position; + } + uint64_t value; + const auto status = std::from_chars(num.data() + position, num.data() + num.size(), value, fromBase); + return status.ec == std::errc::result_out_of_range; + } + + VeloxConvFunction delegate_; + bool ansiEnabled_{false}; +}; + +} // namespace gluten diff --git a/cpp/velox/operators/functions/overlay/README.md b/cpp/velox/operators/functions/overlay/README.md index 8d8beef088f..48bb89e78f1 100644 --- a/cpp/velox/operators/functions/overlay/README.md +++ b/cpp/velox/operators/functions/overlay/README.md @@ -24,8 +24,9 @@ the overlay takes precedence over the Velox implementation. 1. Implement the function in a header/source file in this directory, following Velox's function authoring APIs (simple function, vector function, aggregate, - or window function). See `Round.h` for a simple-function override and - `Elt.h`/`Elt.cc` for a vector function that is missing in Velox, plus the + or window function). See `Round.h` for a simple-function override, + `Elt.h`/`Elt.cc` for a vector function that is missing in Velox, `Conv.h` for an override that + delegates to the Velox function and only adds the missing ANSI behavior, plus the [Velox scalar functions guide](https://github.com/facebookincubator/velox/blob/main/velox/docs/develop/scalar-functions.rst). 2. Register it in `RegisterFunctionOverlay.cc` inside `registerFunctionOverlay()`. Use the same name Gluten's Substrait plan diff --git a/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc b/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc index 99415c2e4f1..dfe56cf65be 100644 --- a/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc +++ b/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc @@ -16,6 +16,7 @@ */ #include "operators/functions/overlay/RegisterFunctionOverlay.h" +#include "operators/functions/overlay/Conv.h" #include "operators/functions/overlay/Elt.h" #include "operators/functions/overlay/Round.h" #include "velox/functions/lib/RegistrationHelpers.h" @@ -44,11 +45,18 @@ void registerEltFunction() { "elt", eltSignatures(), makeElt, velox::exec::VectorFunctionMetadataBuilder().defaultNullBehavior(false).build()); } +// Velox's conv always lets the conversion overflow, which only matches Spark +// with ANSI mode off. +void registerConvFunction() { + velox::registerFunction({"conv"}); +} + } // namespace void registerFunctionOverlay() { registerRoundFunction(); registerEltFunction(); + registerConvFunction(); } } // namespace gluten diff --git a/cpp/velox/tests/SparkFunctionTest.cc b/cpp/velox/tests/SparkFunctionTest.cc index 6ebe4a57fac..147676e5ba3 100644 --- a/cpp/velox/tests/SparkFunctionTest.cc +++ b/cpp/velox/tests/SparkFunctionTest.cc @@ -43,6 +43,13 @@ class SparkFunctionTest : public SparkFunctionBaseTest { } protected: + std::optional conv( + const std::optional& num, + const std::optional& fromBase, + const std::optional& toBase) { + return evaluateOnce("conv(c0, c1, c2)", num, fromBase, toBase); + } + template void runRoundTest(const std::vector>& data) { auto result = evaluate>("round(c0)", makeRowVector({makeFlatVector(data)})); @@ -220,3 +227,51 @@ TEST_F(SparkFunctionTest, eltConstantIndexOverDictionaryInput) { makeFlatVector({"b2", "b0", "b1"}), evaluate("elt(c0, c1, c2)", makeRowVector({index, first, dictionary}))); } + +TEST_F(SparkFunctionTest, convOverflowAnsiOff) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "false"}}); + + // Digits that do not fit in an unsigned 64-bit integer saturate, which is + // what Spark does with ANSI mode off. + EXPECT_EQ(conv("9223372036854775807", 36, 16), "FFFFFFFFFFFFFFFF"); + EXPECT_EQ(conv("10000000000000000", 16, 10), "18446744073709551615"); + EXPECT_EQ(conv("-10000000000000000", 16, -10), "-1"); +} + +TEST_F(SparkFunctionTest, convOverflowAnsiOn) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + + VELOX_ASSERT_THROW(conv("9223372036854775807", 36, 16), "Overflow in function conv()"); + VELOX_ASSERT_THROW(conv("10000000000000000", 16, 10), "Overflow in function conv()"); + VELOX_ASSERT_THROW(conv("-10000000000000000", 16, -10), "Overflow in function conv()"); +} + +TEST_F(SparkFunctionTest, convNoOverflowAnsiOn) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + + EXPECT_EQ(conv("4", 10, 2), "100"); + EXPECT_EQ(conv("big", 36, 16), "3A48"); + // 2^64 - 1 is the largest input that does not overflow. + EXPECT_EQ(conv("18446744073709551615", 10, 10), "18446744073709551615"); + EXPECT_EQ(conv("FFFFFFFFFFFFFFFF", 16, -10), "-1"); + // The sign is applied after the digits are accumulated, so wrapping around + // through a negative input is not an overflow. + EXPECT_EQ(conv("-1", 10, 16), "FFFFFFFFFFFFFFFF"); + EXPECT_EQ(conv("-15", 10, 16), "FFFFFFFFFFFFFFF1"); + // The digits stop at the first character that is invalid for the base, so a + // long tail of invalid characters is not an overflow either. + EXPECT_EQ(conv("11abcabcabcabcabcabcabcabc", 10, 16), "B"); +} + +TEST_F(SparkFunctionTest, convInvalidInputAnsiOn) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + + // Invalid input gives NULL, with ANSI mode on as well. + EXPECT_EQ(conv("15", 1, 10), std::nullopt); + EXPECT_EQ(conv("15", 37, 10), std::nullopt); + EXPECT_EQ(conv("15", 10, 1), std::nullopt); + EXPECT_EQ(conv("15", 10, -37), std::nullopt); + EXPECT_EQ(conv("", 10, 16), std::nullopt); + EXPECT_EQ(conv(" ", 10, 16), std::nullopt); + EXPECT_EQ(conv(std::nullopt, 10, 16), std::nullopt); +} diff --git a/docs/developers/velox-function-development-guide.md b/docs/developers/velox-function-development-guide.md index e4a9e45dd28..c62cc9098c3 100644 --- a/docs/developers/velox-function-development-guide.md +++ b/docs/developers/velox-function-development-guide.md @@ -57,7 +57,9 @@ Use the overlay when: An overlay function follows Spark's ANSI rule the same way Velox's `sparksql` functions do: read `SparkQueryConfig::ansiEnabled()` from the query config, which Gluten populates from the session's `spark.sql.ansi.enabled`. Returning NULL on invalid input with ANSI mode off and raising a user error with it on is what -`elt` does for an out-of-range index. Note that Gluten still falls back on ANSI mode as a whole unless +`elt` does for an out-of-range index. When only the ANSI behavior is missing from an otherwise correct Velox function, +keep delegating to it instead of forking it: `conv` (`overlay/Conv.h`) wraps Velox's `conv` and only adds the overflow +error that ANSI mode requires. Note that Gluten still falls back on ANSI mode as a whole unless `spark.gluten.sql.ansiFallback.enabled` is set to `false`. To add a function: diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala index 48f1b9f19de..792d74eb871 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala @@ -210,6 +210,13 @@ trait SparkPlanExecApi { GenericExpressionTransformer(substraitExprName, children, expr) } + def genConvTransformer( + substraitExprName: String, + children: Seq[ExpressionTransformer], + expr: Conv): ExpressionTransformer = { + GenericExpressionTransformer(substraitExprName, children, expr) + } + def genBase64StaticInvokeTransformer( substraitExprName: String, child: ExpressionTransformer, diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala index 361cf67df1b..1a14c1cd2ed 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala @@ -932,6 +932,12 @@ object ExpressionConverter extends SQLConfHelper with Logging { e.children.map(replaceWithExpressionTransformer0(_, attributeSeq, expressionsMap)), e ) + case c: Conv => + BackendsApiManager.getSparkPlanExecApiInstance.genConvTransformer( + substraitExprName, + c.children.map(replaceWithExpressionTransformer0(_, attributeSeq, expressionsMap)), + c + ) case ce if BackendsApiManager.getSparkPlanExecApiInstance.expressionFlattenSupported(ce) => replaceFlattenedExpressionWithExpressionTransformer( substraitExprName, From 17eeb91d11119011b6610f39fac277462c2f5a99 Mon Sep 17 00:00:00 2001 From: Yuan Date: Mon, 24 Aug 2026 11:43:43 +0100 Subject: [PATCH 3/3] [VL] Support Spark ANSI behavior for element_at, and cover size's ANSI-dependent legacySizeOfNull element_at over an array raises an error for an index past either end of the array when ANSI mode is on, and returns NULL otherwise. Velox's element_at is Presto's SubscriptImpl with allowOutOfBound fixed to true, so it always returns NULL. Add an overlay override that picks the instantiation matching the session's ANSI mode. The two remaining behaviors need no change and are kept: an index of 0 is an error whatever the ANSI mode is, which Velox already reports as "SQL array indices start at 1", and a key a map does not contain gives NULL, because the map side of SubscriptImpl does not look at allowOutOfBound and Spark's ElementAt does not pass failOnError to its map branch. GetMapValue, which Gluten also lowers to element_at, has had no failOnError since Spark 3.4, so it is unaffected too. On the Scala side, fall back for an array input when ElementAt.failOnError disagrees with the session's spark.sql.ansi.enabled, as for elt and conv, and also when defaultValueOutOfBound is set, since Velox has no way to return a default instead of NULL. Size needs no code change: Spark's legacySizeOfNull is 'spark.sql.legacy.sizeOfNull AND NOT ANSI mode', evaluated at analysis time, and ExpressionConverter already forwards Size.legacySizeOfNull to Velox's size(collection, legacySizeOfNull) as a literal, so the ANSI-dependent value is carried by the plan rather than re-derived natively. Add the tests that were missing to lock that in, over both values of spark.sql.legacy.sizeOfNull and both ANSI modes. Note that GetArrayItem, the 0-based array[i] operator lowered to Velox's get, has the same gap as element_at had: it throws invalidArrayIndexError under ANSI mode for an out-of-bound or negative index, while Velox's get returns NULL. It needs both allowOutOfBound and allowNegativeIndices flipped, and is left for a follow-up. The ANSI-on test suite added for elt is renamed to ScalarFunctionsValidateSuiteAnsiOn, matching MathFunctionsValidateSuiteAnsiOn, so it can host the ANSI counterparts of ScalarFunctionsValidateSuite. Co-Authored-By: Claude Opus 5 --- .../velox/VeloxSparkPlanExecApi.scala | 28 ++++++++ .../expression/ExpressionRestrictions.scala | 17 +++++ .../ScalarFunctionsValidateSuite.scala | 44 ++++++++++++ ... ScalarFunctionsValidateSuiteAnsiOn.scala} | 53 +++++++++++++- cpp/velox/CMakeLists.txt | 1 + .../operators/functions/overlay/ElementAt.cc | 71 +++++++++++++++++++ .../operators/functions/overlay/ElementAt.h | 43 +++++++++++ .../overlay/RegisterFunctionOverlay.cc | 8 +++ cpp/velox/tests/SparkFunctionTest.cc | 62 ++++++++++++++++ .../velox-function-development-guide.md | 5 +- .../gluten/backendsapi/SparkPlanExecApi.scala | 7 ++ .../expression/ExpressionConverter.scala | 6 ++ 12 files changed, 342 insertions(+), 3 deletions(-) rename backends-velox/src/test/scala/org/apache/gluten/functions/{StringAnsiValidateSuite.scala => ScalarFunctionsValidateSuiteAnsiOn.scala} (52%) create mode 100644 cpp/velox/operators/functions/overlay/ElementAt.cc create mode 100644 cpp/velox/operators/functions/overlay/ElementAt.h diff --git a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala index 99d605a82c5..36705f280ac 100644 --- a/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala +++ b/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala @@ -1284,6 +1284,34 @@ class VeloxSparkPlanExecApi extends SparkPlanExecApi with Logging { GenericExpressionTransformer(substraitExprName, children, expr) } + override def genElementAtTransformer( + substraitExprName: String, + children: Seq[ExpressionTransformer], + expr: ElementAt): ExpressionTransformer = { + // Only the array input reads failOnError: Spark returns NULL for a key a map does not + // contain whatever the ANSI mode is, and so does Velox. + if (expr.left.dataType.isInstanceOf[ArrayType]) { + if (expr.defaultValueOutOfBound.isDefined) { + GlutenExceptionUtil + .throwsNotFullySupported( + ExpressionNames.ELEMENT_AT, + ElementAtRestrictions.NOT_SUPPORT_DEFAULT_VALUE_OUT_OF_BOUND + ) + } + // Velox derives whether an out-of-bound index raises an error from the session's + // 'spark.sql.ansi.enabled', while Spark captures it in ElementAt.failOnError at + // analysis time. The two normally agree; fall back when they don't. + if (expr.failOnError != SQLConf.get.ansiEnabled) { + GlutenExceptionUtil + .throwsNotFullySupported( + ExpressionNames.ELEMENT_AT, + ElementAtRestrictions.NOT_SUPPORT_FAIL_ON_ERROR_MISMATCH + ) + } + } + GenericExpressionTransformer(substraitExprName, children, expr) + } + override def genBase64StaticInvokeTransformer( substraitExprName: String, child: ExpressionTransformer, diff --git a/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala b/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala index d9f0d180aa8..8eab7591d8d 100644 --- a/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala +++ b/backends-velox/src/main/scala/org/apache/gluten/expression/ExpressionRestrictions.scala @@ -105,6 +105,22 @@ object ConvRestrictions extends ExpressionRestrictions { override val restrictionMessages: Array[String] = Array(NOT_SUPPORT_ANSI_ENABLED_MISMATCH) } +object ElementAtRestrictions extends ExpressionRestrictions { + val NOT_SUPPORT_FAIL_ON_ERROR_MISMATCH: String = + s"${ExpressionNames.ELEMENT_AT} over an array whose failOnError disagrees with the " + + s"session's '${SQLConf.ANSI_ENABLED.key}' is not supported, since Velox derives the " + + s"ANSI behavior of element_at from the session config" + + val NOT_SUPPORT_DEFAULT_VALUE_OUT_OF_BOUND: String = + s"${ExpressionNames.ELEMENT_AT} with a default value for an out-of-bound index is not " + + s"supported in Velox, which always returns NULL for such an index" + + override val functionName: String = ExpressionNames.ELEMENT_AT + + override val restrictionMessages: Array[String] = + Array(NOT_SUPPORT_FAIL_ON_ERROR_MISMATCH, NOT_SUPPORT_DEFAULT_VALUE_OUT_OF_BOUND) +} + object Base64Restrictions extends ExpressionRestrictions { val NOT_SUPPORT_DISABLE_CHUNK_BASE64_STRING: String = s"${ExpressionNames.BASE64} with chunkBase64String disabled is not supported" @@ -149,6 +165,7 @@ object ExpressionRestrictions { Base64Restrictions, EltRestrictions, ConvRestrictions, + ElementAtRestrictions, FormatNumberRestrictions ) } diff --git a/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala b/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala index 19f60238a6b..ccb4f79402f 100644 --- a/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala +++ b/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala @@ -197,6 +197,50 @@ class ScalarFunctionsValidateSuite extends FunctionsValidateSuite { } } + test("element_at") { + withSQLConf(SQLConf.ANSI_ENABLED.key -> "false") { + // An index past either end of the array gives NULL with ANSI mode off, and a key + // the map does not contain gives NULL whatever the ANSI mode is. + runQueryAndCompare( + "SELECT element_at(array(l_orderkey, l_partkey), 1)," + + " element_at(array(l_orderkey, l_partkey), -1)," + + " element_at(array(l_orderkey, l_partkey), 3)," + + " element_at(array(l_orderkey, l_partkey), -3)," + + " element_at(map(1, 'a', 2, 'b'), 3) FROM lineitem") { + checkGlutenPlan[ProjectExecTransformer] + } + + // An index of 0 is an error whatever the ANSI mode is. + intercept[SparkException] { + sql("SELECT element_at(array(l_orderkey, l_partkey), 0) FROM lineitem").collect() + } + } + } + + test("size") { + withTempPath { + path => + Seq[Seq[Integer]](Seq(1, 2, 3), Seq.empty, null) + .toDF("i") + .write + .parquet(path.getCanonicalPath) + spark.read.parquet(path.getCanonicalPath).createOrReplaceTempView("size_tbl") + + withSQLConf(SQLConf.ANSI_ENABLED.key -> "false") { + // With ANSI mode off, spark.sql.legacy.sizeOfNull decides between -1 and NULL + // for a null collection. + Seq("true", "false").foreach { + legacySizeOfNull => + withSQLConf(SQLConf.LEGACY_SIZE_OF_NULL.key -> legacySizeOfNull) { + runQueryAndCompare("SELECT size(i), cardinality(i) FROM size_tbl") { + checkGlutenPlan[ProjectExecTransformer] + } + } + } + } + } + } + test("shiftright") { runQueryAndCompare("SELECT shiftright(int_field1, 1) from datatab limit 1") { checkGlutenPlan[ProjectExecTransformer] diff --git a/backends-velox/src/test/scala/org/apache/gluten/functions/StringAnsiValidateSuite.scala b/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuiteAnsiOn.scala similarity index 52% rename from backends-velox/src/test/scala/org/apache/gluten/functions/StringAnsiValidateSuite.scala rename to backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuiteAnsiOn.scala index 0c14f6dbad0..bbd8f2d017d 100644 --- a/backends-velox/src/test/scala/org/apache/gluten/functions/StringAnsiValidateSuite.scala +++ b/backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuiteAnsiOn.scala @@ -23,10 +23,12 @@ import org.apache.spark.SparkConf import org.apache.spark.SparkException import org.apache.spark.sql.internal.SQLConf -class StringAnsiValidateSuite extends FunctionsValidateSuite { +class ScalarFunctionsValidateSuiteAnsiOn extends FunctionsValidateSuite { disableFallbackCheck + import testImplicits._ + override protected def sparkConf: SparkConf = { super.sparkConf .set(GlutenConfig.GLUTEN_ANSI_FALLBACK_ENABLED.key, "false") @@ -53,4 +55,53 @@ class StringAnsiValidateSuite extends FunctionsValidateSuite { sql("SELECT elt(int_field1 + 3, 'a', 'b', 'c') FROM datatab").collect() } } + + test("element_at") { + // In-bound indices, including the negative ones counting from the end of the array, + // are unaffected by ANSI mode. + runQueryAndCompare( + "SELECT element_at(array(l_orderkey, l_partkey), 1)," + + " element_at(array(l_orderkey, l_partkey), -1) FROM lineitem") { + checkGlutenPlan[ProjectExecTransformer] + } + + // A key the map does not contain gives NULL, in ANSI mode as well. + runQueryAndCompare("SELECT element_at(map(1, 'a', 2, 'b'), 3) FROM lineitem") { + checkGlutenPlan[ProjectExecTransformer] + } + + // An index past either end of the array raises an error in ANSI mode. + intercept[SparkException] { + sql("SELECT element_at(array(l_orderkey, l_partkey), 3) FROM lineitem").collect() + } + intercept[SparkException] { + sql("SELECT element_at(array(l_orderkey, l_partkey), -3) FROM lineitem").collect() + } + // An index of 0 is an error whatever the ANSI mode is. + intercept[SparkException] { + sql("SELECT element_at(array(l_orderkey, l_partkey), 0) FROM lineitem").collect() + } + } + + test("size") { + withTempPath { + path => + Seq[Seq[Integer]](Seq(1, 2, 3), Seq.empty, null) + .toDF("i") + .write + .parquet(path.getCanonicalPath) + spark.read.parquet(path.getCanonicalPath).createOrReplaceTempView("size_tbl") + + // Spark's legacySizeOfNull is 'spark.sql.legacy.sizeOfNull AND NOT ANSI mode', so + // size(null) is NULL here whatever spark.sql.legacy.sizeOfNull says. + Seq("true", "false").foreach { + legacySizeOfNull => + withSQLConf(SQLConf.LEGACY_SIZE_OF_NULL.key -> legacySizeOfNull) { + runQueryAndCompare("SELECT size(i) FROM size_tbl") { + checkGlutenPlan[ProjectExecTransformer] + } + } + } + } + } } diff --git a/cpp/velox/CMakeLists.txt b/cpp/velox/CMakeLists.txt index 2ad8680815b..078f77d6e0e 100644 --- a/cpp/velox/CMakeLists.txt +++ b/cpp/velox/CMakeLists.txt @@ -176,6 +176,7 @@ set(VELOX_SRCS memory/VeloxMemoryManager.cc operators/functions/RegistrationAllFunctions.cc operators/functions/delta/DeltaBitmapAggregator.cc + operators/functions/overlay/ElementAt.cc operators/functions/overlay/Elt.cc operators/functions/overlay/RegisterFunctionOverlay.cc operators/functions/RowConstructorWithNull.cc diff --git a/cpp/velox/operators/functions/overlay/ElementAt.cc b/cpp/velox/operators/functions/overlay/ElementAt.cc new file mode 100644 index 00000000000..d1886c4d732 --- /dev/null +++ b/cpp/velox/operators/functions/overlay/ElementAt.cc @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include "operators/functions/overlay/ElementAt.h" + +#include "velox/functions/lib/SubscriptUtil.h" +#include "velox/functions/sparksql/SparkQueryConfig.h" + +using namespace facebook::velox; + +namespace gluten { +namespace { + +/// Spark's element_at over an array is 1-based, negative indices count from the +/// end of the array, and an index of 0 is an error. Those match Velox's +/// element_at; 'allowOutOfBound' is the one thing ANSI mode changes, an index +/// past the end of the array giving NULL rather than an error. +/// +/// The map side of SubscriptImpl does not look at 'allowOutOfBound', so a key +/// that the map does not contain keeps giving NULL, which is what Spark does in +/// ANSI mode as well. +template +using ElementAtFunction = functions::SubscriptImpl< + /*allowNegativeIndices=*/true, + /*nullOnNegativeIndices=*/false, + allowOutOfBound, + /*indexStartsAtOne=*/true>; + +} // namespace + +std::vector elementAtSignatures() { + // The signatures do not depend on the out-of-bound behavior. + return ElementAtFunction::signatures(); +} + +std::shared_ptr makeElementAt( + const std::string& /*name*/, + const std::vector& inputArgs, + const core::QueryConfig& config) { + VELOX_CHECK_EQ(inputArgs.size(), 2); + if (!inputArgs[0].type->isArray()) { + // Same as Velox's element_at over a map, which may cache a materialized + // version of the map when it is reused across batches. + return std::make_shared>(config.isExpressionEvaluationCacheEnabled()); + } + // The array side holds no state, so one shared instance per behavior is + // enough. Caching is a map-only optimization. + if (functions::sparksql::SparkQueryConfig{config}.ansiEnabled()) { + static const auto kFailOnOutOfBound = + std::make_shared>(/*allowCaching=*/false); + return kFailOnOutOfBound; + } + static const auto kNullOnOutOfBound = + std::make_shared>(/*allowCaching=*/false); + return kNullOnOutOfBound; +} + +} // namespace gluten diff --git a/cpp/velox/operators/functions/overlay/ElementAt.h b/cpp/velox/operators/functions/overlay/ElementAt.h new file mode 100644 index 00000000000..5dc5537d2a2 --- /dev/null +++ b/cpp/velox/operators/functions/overlay/ElementAt.h @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +#include "velox/expression/VectorFunction.h" + +namespace gluten { + +/// Signatures of Spark's element_at: element_at(array(T), integer|bigint) -> T +/// and element_at(map(K, V), K) -> V. +std::vector elementAtSignatures(); + +/// Creates Spark's element_at. +/// +/// Overrides Velox's element_at, which always returns NULL for an index past +/// the end of an array. Spark only does that with ANSI mode off; with ANSI mode +/// on it raises an error. An index of 0 is an error either way, and a key that +/// a map does not contain gives NULL either way, so those are unchanged. +std::shared_ptr makeElementAt( + const std::string& name, + const std::vector& inputArgs, + const facebook::velox::core::QueryConfig& config); + +} // namespace gluten diff --git a/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc b/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc index dfe56cf65be..428a167cd28 100644 --- a/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc +++ b/cpp/velox/operators/functions/overlay/RegisterFunctionOverlay.cc @@ -17,6 +17,7 @@ #include "operators/functions/overlay/RegisterFunctionOverlay.h" #include "operators/functions/overlay/Conv.h" +#include "operators/functions/overlay/ElementAt.h" #include "operators/functions/overlay/Elt.h" #include "operators/functions/overlay/Round.h" #include "velox/functions/lib/RegistrationHelpers.h" @@ -51,12 +52,19 @@ void registerConvFunction() { velox::registerFunction({"conv"}); } +// Velox's element_at always returns NULL for an index past the end of an array, +// which only matches Spark with ANSI mode off. +void registerElementAtFunction() { + velox::exec::registerStatefulVectorFunction("element_at", elementAtSignatures(), makeElementAt); +} + } // namespace void registerFunctionOverlay() { registerRoundFunction(); registerEltFunction(); registerConvFunction(); + registerElementAtFunction(); } } // namespace gluten diff --git a/cpp/velox/tests/SparkFunctionTest.cc b/cpp/velox/tests/SparkFunctionTest.cc index 147676e5ba3..3c3864c02d1 100644 --- a/cpp/velox/tests/SparkFunctionTest.cc +++ b/cpp/velox/tests/SparkFunctionTest.cc @@ -275,3 +275,65 @@ TEST_F(SparkFunctionTest, convInvalidInputAnsiOn) { EXPECT_EQ(conv(" ", 10, 16), std::nullopt); EXPECT_EQ(conv(std::nullopt, 10, 16), std::nullopt); } + +TEST_F(SparkFunctionTest, elementAtArrayOutOfBoundAnsiOff) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "false"}}); + auto array = makeArrayVector({{1, 2, 3}, {1, 2, 3}, {1, 2, 3}, {1, 2, 3}, {1, 2, 3}}); + auto index = makeFlatVector({1, 3, -1, 4, -4}); + + // An index past either end of the array gives NULL with ANSI mode off. + facebook::velox::test::assertEqualVectors( + makeNullableFlatVector({1, 3, 3, std::nullopt, std::nullopt}), + evaluate("element_at(c0, c1)", makeRowVector({array, index}))); +} + +TEST_F(SparkFunctionTest, elementAtArrayOutOfBoundAnsiOn) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + auto array = makeArrayVector({{1, 2, 3}, {1, 2, 3}}); + + // In-bound indices, including the negative ones counting from the end, are + // unaffected by ANSI mode. + facebook::velox::test::assertEqualVectors( + makeFlatVector({1, 3}), + evaluate("element_at(c0, c1)", makeRowVector({array, makeFlatVector({1, -1})}))); + + VELOX_ASSERT_THROW( + evaluate("element_at(c0, c1)", makeRowVector({array, makeFlatVector({1, 4})})), + "Array subscript index out of bounds"); + VELOX_ASSERT_THROW( + evaluate("element_at(c0, c1)", makeRowVector({array, makeFlatVector({-4, 1})})), + "Array subscript index out of bounds"); +} + +TEST_F(SparkFunctionTest, elementAtArrayZeroIndex) { + auto data = makeRowVector({makeArrayVector({{1, 2, 3}}), makeFlatVector({0})}); + + // An index of 0 is an error whatever the ANSI mode is. + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "false"}}); + VELOX_ASSERT_THROW(evaluate("element_at(c0, c1)", data), "SQL array indices start at 1"); + + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + VELOX_ASSERT_THROW(evaluate("element_at(c0, c1)", data), "SQL array indices start at 1"); +} + +TEST_F(SparkFunctionTest, elementAtMapMissingKeyAnsiOn) { + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + auto map = makeMapVector({{{1, 10}, {2, 20}}, {{1, 10}, {2, 20}}}); + auto key = makeFlatVector({2, 3}); + + // A key the map does not contain gives NULL, with ANSI mode on as well. + facebook::velox::test::assertEqualVectors( + makeNullableFlatVector({20, std::nullopt}), evaluate("element_at(c0, c1)", makeRowVector({map, key}))); +} + +TEST_F(SparkFunctionTest, sizeOfNull) { + // Gluten passes Spark's Size.legacySizeOfNull as the second argument, and + // Spark derives it from 'spark.sql.legacy.sizeOfNull' AND NOT ANSI mode, so + // both behaviors have to work regardless of the session's ANSI mode. + queryCtx_->testingOverrideConfigUnsafe({{sparkAnsiEnabledConfigKey(), "true"}}); + auto data = makeRowVector({makeArrayVectorFromJson({"[1, 2, 3]", "null"})}); + + facebook::velox::test::assertEqualVectors( + makeNullableFlatVector({3, std::nullopt}), evaluate("size(c0, false)", data)); + facebook::velox::test::assertEqualVectors(makeFlatVector({3, -1}), evaluate("size(c0, true)", data)); +} diff --git a/docs/developers/velox-function-development-guide.md b/docs/developers/velox-function-development-guide.md index c62cc9098c3..87a40f14503 100644 --- a/docs/developers/velox-function-development-guide.md +++ b/docs/developers/velox-function-development-guide.md @@ -59,8 +59,9 @@ An overlay function follows Spark's ANSI rule the same way Velox's `sparksql` fu `spark.sql.ansi.enabled`. Returning NULL on invalid input with ANSI mode off and raising a user error with it on is what `elt` does for an out-of-range index. When only the ANSI behavior is missing from an otherwise correct Velox function, keep delegating to it instead of forking it: `conv` (`overlay/Conv.h`) wraps Velox's `conv` and only adds the overflow -error that ANSI mode requires. Note that Gluten still falls back on ANSI mode as a whole unless -`spark.gluten.sql.ansiFallback.enabled` is set to `false`. +error that ANSI mode requires, and `element_at` (`overlay/ElementAt.h`) just picks the Velox `SubscriptImpl` +instantiation that raises an error for an out-of-bound array index. Note that Gluten still falls back on ANSI mode as a +whole unless `spark.gluten.sql.ansiFallback.enabled` is set to `false`. To add a function: 1. Implement it in a file under `cpp/velox/operators/functions/overlay/`, using Velox's function authoring APIs (simple function, diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala index 792d74eb871..e9399344273 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala @@ -217,6 +217,13 @@ trait SparkPlanExecApi { GenericExpressionTransformer(substraitExprName, children, expr) } + def genElementAtTransformer( + substraitExprName: String, + children: Seq[ExpressionTransformer], + expr: ElementAt): ExpressionTransformer = { + GenericExpressionTransformer(substraitExprName, children, expr) + } + def genBase64StaticInvokeTransformer( substraitExprName: String, child: ExpressionTransformer, diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala index 1a14c1cd2ed..18fe954fd2e 100644 --- a/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala +++ b/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala @@ -938,6 +938,12 @@ object ExpressionConverter extends SQLConfHelper with Logging { c.children.map(replaceWithExpressionTransformer0(_, attributeSeq, expressionsMap)), c ) + case ea: ElementAt => + BackendsApiManager.getSparkPlanExecApiInstance.genElementAtTransformer( + substraitExprName, + ea.children.map(replaceWithExpressionTransformer0(_, attributeSeq, expressionsMap)), + ea + ) case ce if BackendsApiManager.getSparkPlanExecApiInstance.expressionFlattenSupported(ce) => replaceFlattenedExpressionWithExpressionTransformer( substraitExprName,