Skip to content

[VL] Add ANSI mode support for Spark CAST(NumericType as integral) #11854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
minni31 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[VL] Add ANSI mode support for Spark CAST(NumericType as integral) #11854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50b5ceb
feat: Add ANSI mode support for Spark CAST(NumericType as integral)
Mar 30, 2026
026cc0e
Merge branch 'main' into oss/ansi-cast-numeric-to-integral
minni31 Mar 31, 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
3 changes: 3 additions & 0 deletions cpp/velox/compute/WholeStageResultIterator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,9 @@ std::unordered_map<std::string, std::string> WholeStageResultIterator::getQueryC

configs[velox::core::QueryConfig::kSparkPartitionId] = std::to_string(taskInfo_.partitionId);

configs[velox::core::QueryConfig::kSparkAnsiModeEnabled] =
std::to_string(veloxCfg_->get<bool>(kVeloxSparkAnsiModeEnabled, false));

// Enable Spark legacy date formatter if spark.sql.legacy.timeParserPolicy is set to 'LEGACY'
// or 'legacy'
if (veloxCfg_->get<std::string>(kSparkLegacyTimeParserPolicy, "") == "LEGACY") {
Expand Down
2 changes: 2 additions & 0 deletions cpp/velox/config/VeloxConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ const std::string kQueryTraceTaskRegExp = "spark.gluten.sql.columnar.backend.vel
const std::string kOpTraceDirectoryCreateConfig =
"spark.gluten.sql.columnar.backend.velox.opTraceDirectoryCreateConfig";

const std::string kVeloxSparkAnsiModeEnabled = "spark.sql.ansi.enabled";
Copy link
Copy Markdown
Contributor

@weiting-chen weiting-chen Apr 17, 2026

Choose a reason for hiding this comment

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

Redundant config constant - ANSI mode is already propagated

Problem: This new constant kVeloxSparkAnsiModeEnabled duplicates the existing kAnsiEnabled = "spark.sql.ansi.enabled" already defined in cpp/core/config/GlutenConfig.h. Furthermore, the corresponding config propagation added in WholeStageResultIterator.cc references velox::core::QueryConfig::kSparkAnsiModeEnabled, which does not exist in the currently pinned Velox version (the existing constant is kSparkAnsiEnabled). This is the root cause of the build-native-lib-centos-7 CI failure.

Evidence:
`cpp
// Already exists on main (GlutenConfig.h):
const std::string kAnsiEnabled = "spark.sql.ansi.enabled";

// Already exists on main (WholeStageResultIterator.cc line 576):
configs[velox::core::QueryConfig::kSparkAnsiEnabled] =
veloxCfg_->getstd::string(kAnsiEnabled, "false");

// This PR adds (duplicate + wrong constant name):
const std::string kVeloxSparkAnsiModeEnabled = "spark.sql.ansi.enabled";
configs[velox::core::QueryConfig::kSparkAnsiModeEnabled] = ... // does not exist!
`

Suggested Fix: Remove both C++ changes entirely (this file and the WholeStageResultIterator.cc change). The existing infrastructure in GlutenConfig.h already handles ANSI mode propagation to Velox.


// Cudf config.
// GPU RMM memory resource
const std::string kCudfMemoryResource = "spark.gluten.sql.columnar.backend.velox.cudf.memoryResource";
Expand Down
18 changes: 8 additions & 10 deletions gluten-substrait/src/main/java/org/apache/gluten/substrait/expression/CastNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,24 @@ public class CastNode implements ExpressionNode, Serializable {
private final TypeNode typeNode;
private final ExpressionNode expressionNode;

public final boolean isTryCast;
// Substrait Cast FailureBehavior:
// 0 = UNSPECIFIED (Spark LEGACY: allow overflow/truncation)
// 1 = RETURN_NULL (Spark TRY: return null on failure)
// 2 = THROW_EXCEPTION (Spark ANSI: throw on overflow)
public final int failureBehavior;

CastNode(TypeNode typeNode, ExpressionNode expressionNode, boolean isTryCast) {
CastNode(TypeNode typeNode, ExpressionNode expressionNode, int failureBehavior) {
this.typeNode = typeNode;
this.expressionNode = expressionNode;
this.isTryCast = isTryCast;
this.failureBehavior = failureBehavior;
}

@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);
}
castBuilder.setFailureBehaviorValue(failureBehavior);
Expression.Builder builder = Expression.newBuilder();
builder.setCast(castBuilder.build());
return builder.build();
Expand Down
16 changes: 15 additions & 1 deletion gluten-substrait/src/main/java/org/apache/gluten/substrait/expression/ExpressionBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,21 @@ public static AggregateFunctionNode makeAggregateFunction(

public static CastNode makeCast(
TypeNode typeNode, ExpressionNode expressionNode, boolean isTryCast) {
Copy link
Copy Markdown
Contributor

@weiting-chen weiting-chen Apr 17, 2026

Choose a reason for hiding this comment

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

Inconsistency between 3-arg and 4-arg makeCast overloads

Problem: The backward-compatible 3-arg overload maps isTryCast=false to THROW_EXCEPTION(2), while the new 4-arg overload (used by CastTransformer) maps the same semantic intent (non-TRY, non-ANSI legacy cast) to UNSPECIFIED(0). Both produce identical Velox runtime behavior (verified in SubstraitToVeloxExpr.cc - Velox treats UNSPECIFIED and THROW_EXCEPTION the same for regular casts), but the inconsistency is confusing for future maintainers.

Evidence:
`java
// 3-arg: non-TRY -> THROW_EXCEPTION (2)
return new CastNode(typeNode, expressionNode, isTryCast ? 1 : 2);

// 4-arg: non-TRY, non-ANSI -> UNSPECIFIED (0)
failureBehavior = 0; // UNSPECIFIED (legacy)
`

Suggested Fix: Align the 3-arg overload to also use UNSPECIFIED(0) for non-TRY casts:
java public static CastNode makeCast( TypeNode typeNode, ExpressionNode expressionNode, boolean isTryCast) { return new CastNode(typeNode, expressionNode, isTryCast ? 1 : 0); }
Note: verify ClickHouse backend doesn't depend on receiving THROW_EXCEPTION from this overload before making this change.

return new CastNode(typeNode, expressionNode, isTryCast);
// Backward-compatible: isTryCast=true → RETURN_NULL(1), false → THROW_EXCEPTION(2)
return new CastNode(typeNode, expressionNode, isTryCast ? 1 : 2);
}

public static CastNode makeCast(
TypeNode typeNode, ExpressionNode expressionNode, boolean isTryCast, boolean isAnsiCast) {
int failureBehavior;
if (isTryCast) {
failureBehavior = 1; // RETURN_NULL
} else if (isAnsiCast) {
failureBehavior = 2; // THROW_EXCEPTION
} else {
failureBehavior = 0; // UNSPECIFIED (legacy)
}
return new CastNode(typeNode, expressionNode, failureBehavior);
}

public static StringMapNode makeStringMap(Map<String, String> values) {
Expand Down
4 changes: 3 additions & 1 deletion ...en-substrait/src/main/scala/org/apache/gluten/expression/UnaryExpressionTransformer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@ case class CastTransformer(substraitExprName: String, child: ExpressionTransform
extends UnaryExpressionTransformer {
override def doTransform(context: SubstraitContext): ExpressionNode = {
val typeNode = ConverterUtils.getTypeNode(dataType, original.nullable)
val shims = SparkShimLoader.getSparkShims
ExpressionBuilder.makeCast(
typeNode,
child.doTransform(context),
SparkShimLoader.getSparkShims.withTryEvalMode(original))
shims.withTryEvalMode(original),
shims.withAnsiEvalMode(original))
}
}

Expand Down
4 changes: 0 additions & 4 deletions gluten-ut/spark34/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,6 @@ class VeloxTestSettings extends BackendTestSettings {
"Process Infinity, -Infinity, NaN in case insensitive manner" // +inf not supported in folly.
)
.exclude("cast from timestamp II") // Rewrite test for Gluten not supported with ANSI mode
.exclude("ANSI mode: Throw exception on casting out-of-range value to byte type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to short type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to int type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to long type")
.exclude("cast from invalid string to numeric should throw NumberFormatException")
.exclude("SPARK-26218: Fix the corner case of codegen when casting float to Integer")
// Set timezone through config.
Expand Down
5 changes: 0 additions & 5 deletions gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,6 @@ class VeloxTestSettings extends BackendTestSettings {
.exclude(
"Process Infinity, -Infinity, NaN in case insensitive manner" // +inf not supported in folly.
)
Copy link
Copy Markdown
Contributor

@weiting-chen weiting-chen Apr 17, 2026

Choose a reason for hiding this comment

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

Unrelated exclusion removal - "cast from timestamp II" removed only in spark35

Problem: This spark35 VeloxTestSettings change removes the "cast from timestamp II" exclusion in addition to the 4 ANSI overflow tests. The spark34 and spark40 versions of this file keep this exclusion. This appears unrelated to the ANSI numeric cast feature.

Investigation Needed: Is this removal intentional? If so, please add a note in the PR description explaining why it's included. If accidental, please revert this line and handle it in a separate PR.

.exclude("cast from timestamp II") // Rewrite test for Gluten not supported with ANSI mode
.exclude("ANSI mode: Throw exception on casting out-of-range value to byte type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to short type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to int type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to long type")
.exclude("cast from invalid string to numeric should throw NumberFormatException")
.exclude("SPARK-26218: Fix the corner case of codegen when casting float to Integer")
// Set timezone through config.
Expand Down
4 changes: 0 additions & 4 deletions gluten-ut/spark40/src/test/scala/org/apache/gluten/utils/velox/VeloxTestSettings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ class VeloxTestSettings extends BackendTestSettings {
"Process Infinity, -Infinity, NaN in case insensitive manner" // +inf not supported in folly.
)
.exclude("cast from timestamp II") // Rewrite test for Gluten not supported with ANSI mode
.exclude("ANSI mode: Throw exception on casting out-of-range value to byte type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to short type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to int type")
.exclude("ANSI mode: Throw exception on casting out-of-range value to long type")
.exclude("cast from invalid string to numeric should throw NumberFormatException")
.exclude("SPARK-26218: Fix the corner case of codegen when casting float to Integer")
// Set timezone through config.
Expand Down
4 changes: 2 additions & 2 deletions ...spark40/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenTryCastSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
package org.apache.spark.sql.catalyst.expressions

import org.apache.spark.sql.GlutenTestsTrait
import org.apache.spark.sql.catalyst.util.DateTimeTestUtils.{withDefaultTimeZone, ALL_TIMEZONES, UTC, UTC_OPT}
import org.apache.spark.sql.catalyst.util.DateTimeUtils.{fromJavaTimestamp, millisToMicros, TimeZoneUTC}
import org.apache.spark.sql.catalyst.util.DateTimeTestUtils.{ALL_TIMEZONES, UTC, UTC_OPT, withDefaultTimeZone}
import org.apache.spark.sql.catalyst.util.DateTimeUtils.{TimeZoneUTC, fromJavaTimestamp, millisToMicros}
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
import org.apache.spark.util.DebuggableThreadUtils
Expand Down
1 change: 1 addition & 0 deletions shims/spark34/src/main/scala/org/apache/gluten/sql/shims/spark34/Spark34Shims.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ class Spark34Shims extends SparkShims {
case d: Divide => d.evalMode == EvalMode.ANSI
case m: Multiply => m.evalMode == EvalMode.ANSI
case i: IntegralDivide => i.evalMode == EvalMode.ANSI
case c: Cast => c.evalMode == EvalMode.ANSI
case _ => false
}
}
Expand Down
1 change: 1 addition & 0 deletions shims/spark35/src/main/scala/org/apache/gluten/sql/shims/spark35/Spark35Shims.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ class Spark35Shims extends SparkShims {
case d: Divide => d.evalMode == EvalMode.ANSI
case m: Multiply => m.evalMode == EvalMode.ANSI
case i: IntegralDivide => i.evalMode == EvalMode.ANSI
case c: Cast => c.evalMode == EvalMode.ANSI
case _ => false
}
}
Expand Down
Loading