Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
10414a2
Preserve store assignment cast modes
ReemaAlzaid May 7, 2026
44d0731
Address Velox insert review comments
ReemaAlzaid May 14, 2026
6e4b609
Add store assignment policy insert tests
ReemaAlzaid May 14, 2026
0b6c65a
Preserve store assignment cast modes
ReemaAlzaid May 7, 2026
222f1f1
Address Velox insert review comments
ReemaAlzaid May 14, 2026
a998015
Add store assignment policy insert tests
ReemaAlzaid May 14, 2026
eea937a
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid May 27, 2026
b2f41b5
Merge branch 'apache:main' into store-assignment-policy-cast-modes
ReemaAlzaid Jun 1, 2026
89f3284
Merge branch 'store-assignment-policy-cast-modes' of https://github.c…
ReemaAlzaid Jun 8, 2026
8e1a1cb
Use upstream Velox cast mode special forms
ReemaAlzaid Jun 8, 2026
36133ca
Use Velox ANSI cast support helper
ReemaAlzaid Jun 16, 2026
44eb11f
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Jun 16, 2026
2de3e9e
Format Spark cast mode changes
ReemaAlzaid Jun 16, 2026
ec071d3
Merge branch 'store-assignment-policy-cast-modes' of https://github.c…
ReemaAlzaid Jun 16, 2026
331bfb9
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Jun 26, 2026
1350edb
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Jul 2, 2026
9cc99a2
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Jul 7, 2026
653285b
Adapt Spark cast mode registration to Velox API
ReemaAlzaid Jul 7, 2026
469c319
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Jul 8, 2026
570e3c2
Fix Spark ANSI config key in cast tests
ReemaAlzaid Jul 8, 2026
a21cffc
Merge branch 'store-assignment-policy-cast-modes' of https://github.c…
ReemaAlzaid Jul 8, 2026
06856ba
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Jul 16, 2026
7dc2bef
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Aug 12, 2026
abb474c
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Aug 12, 2026
83d88df
[GLUTEN-10134][VL] Fix CI failures for store assignment cast modes
ReemaAlzaid Aug 12, 2026
d69e751
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Aug 13, 2026
71c3610
Merge branch 'main' into store-assignment-policy-cast-modes
ReemaAlzaid Aug 13, 2026
dac6f8f
address comments
ReemaAlzaid Aug 13, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cpp/velox/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ set(VELOX_SRCS
memory/VeloxMemoryManager.cc
operators/functions/RegistrationAllFunctions.cc
operators/functions/RowConstructorWithNull.cc
operators/functions/SparkCastModeSpecialForms.cc
operators/functions/SparkExprToSubfieldFilterParser.cc
operators/plannodes/RowVectorStream.cc
operators/hashjoin/HashTableBuilder.cc
Expand Down
2 changes: 2 additions & 0 deletions cpp/velox/operators/functions/RegistrationAllFunctions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "operators/functions/Arithmetic.h"
#include "operators/functions/RowConstructorWithNull.h"
#include "operators/functions/RowFunctionWithNull.h"
#include "operators/functions/SparkCastModeSpecialForms.h"
#include "velox/expression/SpecialFormRegistry.h"
#include "velox/expression/VectorFunction.h"
#include "velox/functions/iceberg/Register.h"
Expand Down Expand Up @@ -83,6 +84,7 @@ void registerFunctionOverwrite() {

void registerAllFunctions() {
velox::functions::sparksql::registerFunctions("");
registerSparkCastModeSpecialForms();
velox::aggregate::prestosql::registerAllAggregateFunctions(
"", true /*registerCompanionFunctions*/, false /*onlyPrestoSignatures*/, true /*overwrite*/);
velox::functions::aggregate::sparksql::registerAggregateFunctions(
Expand Down
115 changes: 115 additions & 0 deletions cpp/velox/operators/functions/SparkCastModeSpecialForms.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* 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/SparkCastModeSpecialForms.h"

#include "velox/expression/SpecialFormRegistry.h"
#include "velox/functions/sparksql/specialforms/SparkCastExpr.h"
#include "velox/functions/sparksql/specialforms/SparkCastHooks.h"

namespace gluten {
namespace {

using namespace facebook::velox;
using facebook::velox::functions::sparksql::SparkCastExpr;
using facebook::velox::functions::sparksql::SparkCastHooks;

bool isIntegralType(const TypePtr& type) {
return type == TINYINT() || type == SMALLINT() || type == INTEGER() ||
type == BIGINT();
}

bool isAnsiSupported(const TypePtr& fromType, const TypePtr& toType) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also a check on the Velox side. Will we maintain the ANSI support check only here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended to mirror the existing Velox ANSI support side check not to replace it, since that check is private today, I added this local helper for the expression level ANSI/legacy cast mode. I’ll add a comment to make sure we keep it aligned with Velox

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we really need it, to reduce code maintenance effort, could we make it public in Velox to allow calling it here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created the pr here facebookincubator/velox#17687

if (fromType->isVarchar()) {
return toType->isBoolean() || toType->isDate() || isIntegralType(toType);
}
return false;
}

exec::ExprPtr makeSparkCastExpr(
const TypePtr& type,
exec::ExprPtr&& input,
bool trackCpuUsage,
bool isTryCast,
bool allowOverflow,
const core::QueryConfig& config) {
return std::make_shared<SparkCastExpr>(
type,
std::move(input),
trackCpuUsage,
isTryCast,
std::make_shared<SparkCastHooks>(config, allowOverflow));
}

class SparkAnsiCastCallToSpecialForm : public exec::CastCallToSpecialForm {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still wondering if we can move or keep these code on Velox side, and just call the register API in Gluten C++ code. Could you please clarify?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Existing Velox Spark cast depends on session ansiEnabled, but Gluten needs Substrait’s per expression cast mode. This patch adds thin Gluten registration while reusing Velox cast code. I agree we can move it to Velox and call the registration API from Gluten

public:
exec::ExprPtr constructSpecialForm(
const TypePtr& type,
std::vector<exec::ExprPtr>&& compiledChildren,
bool trackCpuUsage,
const core::QueryConfig& config) override {
VELOX_CHECK_EQ(
compiledChildren.size(),
1,
"ANSI CAST statements expect exactly 1 argument, received {}.",
compiledChildren.size());

const auto& fromType = compiledChildren[0]->type();
const bool isTryCast = !isAnsiSupported(fromType, type);
return makeSparkCastExpr(
type,
std::move(compiledChildren[0]),
trackCpuUsage,
isTryCast,
isTryCast,
config);
}
};

class SparkLegacyCastCallToSpecialForm : public exec::CastCallToSpecialForm {
public:
exec::ExprPtr constructSpecialForm(
const TypePtr& type,
std::vector<exec::ExprPtr>&& compiledChildren,
bool trackCpuUsage,
const core::QueryConfig& config) override {
VELOX_CHECK_EQ(
compiledChildren.size(),
1,
"LEGACY CAST statements expect exactly 1 argument, received {}.",
compiledChildren.size());

return makeSparkCastExpr(
type,
std::move(compiledChildren[0]),
trackCpuUsage,
true,
true,
config);
}
};

} // namespace

void registerSparkCastModeSpecialForms() {
exec::registerFunctionCallToSpecialForm(
kSparkAnsiCast, std::make_unique<SparkAnsiCastCallToSpecialForm>());
exec::registerFunctionCallToSpecialForm(
kSparkLegacyCast, std::make_unique<SparkLegacyCastCallToSpecialForm>());
}

} // namespace gluten
27 changes: 27 additions & 0 deletions cpp/velox/operators/functions/SparkCastModeSpecialForms.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* 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

namespace gluten {

constexpr const char* kSparkAnsiCast = "spark_ansi_cast";
constexpr const char* kSparkLegacyCast = "spark_legacy_cast";

void registerSparkCastModeSpecialForms();

} // namespace gluten
32 changes: 26 additions & 6 deletions cpp/velox/substrait/SubstraitToVeloxExpr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "SubstraitToVeloxExpr.h"
#include "TypeUtils.h"
#include "operators/functions/SparkCastModeSpecialForms.h"
#include "velox/vector/FlatVector.h"
#include "velox/vector/VariantToVector.h"

Expand Down Expand Up @@ -146,16 +147,24 @@ TypePtr getScalarType(const ::substrait::Expression::Literal& literal) {
}
}

/// Whether is try cast.
bool isTryCast(::substrait::Expression::Cast::FailureBehavior failureBehavior) {
enum class SparkCastMode {
kLegacy,
kAnsi,
kTry,
};

SparkCastMode sparkCastMode(
::substrait::Expression::Cast::FailureBehavior failureBehavior) {
switch (failureBehavior) {
case ::substrait::Expression_Cast_FailureBehavior_FAILURE_BEHAVIOR_UNSPECIFIED:
return SparkCastMode::kLegacy;
case ::substrait::Expression_Cast_FailureBehavior_FAILURE_BEHAVIOR_THROW_EXCEPTION:
return false;
return SparkCastMode::kAnsi;
case ::substrait::Expression_Cast_FailureBehavior_FAILURE_BEHAVIOR_RETURN_NULL:
return true;
return SparkCastMode::kTry;
default:
VELOX_NYI("The given failure behavior is NOT supported: '{}'", std::to_string(failureBehavior));
VELOX_NYI(
"The given failure behavior is NOT supported: '{}'", std::to_string(failureBehavior));
}
}

Expand Down Expand Up @@ -564,7 +573,18 @@ core::TypedExprPtr SubstraitVeloxExprConverter::toVeloxExpr(
const RowTypePtr& inputType) {
auto type = SubstraitParser::parseType(castExpr.type());
std::vector<core::TypedExprPtr> inputs{toVeloxExpr(castExpr.input(), inputType)};
return std::make_shared<core::CastTypedExpr>(type, inputs, isTryCast(castExpr.failure_behavior()));
switch (sparkCastMode(castExpr.failure_behavior())) {
case SparkCastMode::kLegacy:
return std::make_shared<const core::CallTypedExpr>(
type, std::move(inputs), kSparkLegacyCast);
case SparkCastMode::kAnsi:
return std::make_shared<const core::CallTypedExpr>(
type, std::move(inputs), kSparkAnsiCast);
case SparkCastMode::kTry:
return std::make_shared<core::CastTypedExpr>(type, std::move(inputs), true);
default:
VELOX_UNREACHABLE();
}
}

core::TypedExprPtr SubstraitVeloxExprConverter::toVeloxExpr(
Expand Down
33 changes: 33 additions & 0 deletions cpp/velox/tests/SparkFunctionTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
#include <vector>

#include "operators/functions/RegistrationAllFunctions.h"
#include "operators/functions/SparkCastModeSpecialForms.h"
#include "velox/common/base/tests/GTestUtils.h"
#include "velox/core/Expressions.h"
#include "velox/core/QueryConfig.h"
#include "velox/functions/sparksql/tests/SparkFunctionBaseTest.h"

using namespace facebook::velox::functions::sparksql::test;
Expand Down Expand Up @@ -111,3 +115,32 @@ TEST_F(SparkFunctionTest, roundWithDecimal) {
runRoundWithDecimalTest<int16_t>(testRoundWithDecIntegralData<int16_t>());
runRoundWithDecimalTest<int8_t>(testRoundWithDecIntegralData<int8_t>());
}

TEST_F(SparkFunctionTest, expressionLevelAnsiCastIgnoresSessionAnsiOff) {
queryCtx_->testingOverrideConfigUnsafe(
{{core::QueryConfig::kSparkAnsiEnabled, "false"}});
auto input = makeRowVector({makeFlatVector<std::string>({"2147483648"})});
core::TypedExprPtr field =
std::make_shared<const core::FieldAccessTypedExpr>(VARCHAR(), "c0");
auto ansiCast = std::make_shared<const core::CallTypedExpr>(
INTEGER(),
std::vector<core::TypedExprPtr>{field},
gluten::kSparkAnsiCast);

VELOX_ASSERT_THROW(evaluate(ansiCast, input), "Cannot cast");
}

TEST_F(SparkFunctionTest, expressionLevelLegacyCastIgnoresSessionAnsiOn) {
queryCtx_->testingOverrideConfigUnsafe(
{{core::QueryConfig::kSparkAnsiEnabled, "true"}});
auto input = makeRowVector({makeFlatVector<int32_t>({1234567})});
core::TypedExprPtr field =
std::make_shared<const core::FieldAccessTypedExpr>(INTEGER(), "c0");
auto legacyCast = std::make_shared<const core::CallTypedExpr>(
TINYINT(),
std::vector<core::TypedExprPtr>{field},
gluten::kSparkLegacyCast);

facebook::velox::test::assertEqualVectors(
makeFlatVector<int8_t>({-121}), evaluate(legacyCast, input));
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,28 +23,46 @@
import java.io.Serializable;

public class CastNode implements ExpressionNode, Serializable {
public enum CastMode {
LEGACY,
ANSI,
TRY
}

private final TypeNode typeNode;
private final ExpressionNode expressionNode;

public final boolean isTryCast;
public final CastMode castMode;

CastNode(TypeNode typeNode, ExpressionNode expressionNode, boolean isTryCast) {
this(typeNode, expressionNode, isTryCast ? CastMode.TRY : CastMode.ANSI);
}

CastNode(TypeNode typeNode, ExpressionNode expressionNode, CastMode castMode) {
this.typeNode = typeNode;
this.expressionNode = expressionNode;
this.isTryCast = isTryCast;
this.castMode = castMode;
}

@Override
public Expression toProtobuf() {
Expression.Cast.Builder castBuilder = Expression.Cast.newBuilder();
castBuilder.setType(typeNode.toProtobuf());
castBuilder.setInput(expressionNode.toProtobuf());
if (!isTryCast) {
// Throw exception on failure.
castBuilder.setFailureBehaviorValue(2);
} else {
// Return null on failure.
castBuilder.setFailureBehaviorValue(1);
switch (castMode) {
case ANSI:
// Throw exception on failure.
castBuilder.setFailureBehaviorValue(2);
break;
case TRY:
// Return null on failure.
castBuilder.setFailureBehaviorValue(1);
break;
case LEGACY:
// Leave failure behavior unspecified to preserve Spark legacy cast semantics.
break;
default:
throw new IllegalStateException("Unsupported cast mode: " + castMode);
}
Expression.Builder builder = Expression.newBuilder();
builder.setCast(castBuilder.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ public static CastNode makeCast(
return new CastNode(typeNode, expressionNode, isTryCast);
}

public static CastNode makeCast(
TypeNode typeNode, ExpressionNode expressionNode, CastNode.CastMode castMode) {
return new CastNode(typeNode, expressionNode, castMode);
}

public static StringMapNode makeStringMap(Map<String, String> values) {
return new StringMapNode(values);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import org.apache.gluten.sql.shims.SparkShimLoader
import org.apache.gluten.substrait.`type`.ListNode
import org.apache.gluten.substrait.`type`.MapNode
import org.apache.gluten.substrait.SubstraitContext
import org.apache.gluten.substrait.expression.{ExpressionBuilder, ExpressionNode, StructLiteralNode}
import org.apache.gluten.substrait.expression.{CastNode, ExpressionBuilder, ExpressionNode, StructLiteralNode}

import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.types._
Expand All @@ -45,10 +45,19 @@ case class CastTransformer(substraitExprName: String, child: ExpressionTransform
extends UnaryExpressionTransformer {
override def doTransform(context: SubstraitContext): ExpressionNode = {
val typeNode = ConverterUtils.getTypeNode(dataType, original.nullable)
val sparkShims = SparkShimLoader.getSparkShims
// Store-assignment casts can carry EvalMode.ANSI even when session ANSI is disabled.
val castMode = if (sparkShims.withTryEvalMode(original)) {
CastNode.CastMode.TRY
} else if (sparkShims.withAnsiEvalMode(original)) {
CastNode.CastMode.ANSI
} else {
CastNode.CastMode.LEGACY
}
ExpressionBuilder.makeCast(
typeNode,
child.doTransform(context),
SparkShimLoader.getSparkShims.withTryEvalMode(original))
castMode)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,25 @@ class GlutenInsertSuite
}
}

testGluten("storeAssignmentPolicy default ANSI is independent from ANSI mode") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't appear to be a variant of a Spark test. Can we move it to a new suite under backends-velox/?

withTable("store_assignment_ansi") {
withSQLConf(SQLConf.ANSI_ENABLED.key -> "false") {
assert(SQLConf.get.storeAssignmentPolicy == SQLConf.StoreAssignmentPolicy.ANSI)

spark.sql("CREATE TABLE store_assignment_ansi (c INT) USING PARQUET")
intercept[Exception] {
spark.sql("INSERT INTO store_assignment_ansi SELECT '2147483648'").collect()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a check for the exception message to confirm the expected exception is thrown?

}

withSQLConf(
SQLConf.STORE_ASSIGNMENT_POLICY.key -> SQLConf.StoreAssignmentPolicy.LEGACY.toString) {
spark.sql("INSERT INTO store_assignment_ansi SELECT '2147483648'").collect()
checkAnswer(spark.table("store_assignment_ansi"), Row(null))
}
}
}
}

ignoreGluten("Cleanup staging files if job failed") {
// Using a unique table name in this test. Sometimes, the table is not removed for some unknown
// reason, which can cause test failure (location already exists) if other following tests have
Expand Down
Loading
Loading