You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/09/06 03:48:18 UTC
(datafusion) branch main updated: fix(spark): accept every integer width in `factorial`, matching Spark's implicit cast (#24943)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new a5a525e1d6 fix(spark): accept every integer width in `factorial`, matching Spark's implicit cast (#24943)
a5a525e1d6 is described below
commit a5a525e1d67a6edfb9c29b4d470cf45778e872d6
Author: Shayan Gh <98...@users.noreply.github.com>
AuthorDate: Sun Sep 6 03:38:01 2026 +0000
fix(spark): accept every integer width in `factorial`, matching Spark's implicit cast (#24943)
## Which issue does this PR close?
- Closes #24940.
## Rationale for this change
In Spark mode, `SELECT factorial(5)` fails to plan: an untyped integer
literal is `Int64` in
DataFusion and the function's signature was `Exact(Int32)`. Spark's
`Factorial` declares
`inputTypes = Seq(IntegerType)` but extends `ImplicitCastInputTypes`, so
a real Spark casts
TINYINT, SMALLINT and BIGINT arguments to INT before evaluating.
Verified against
`pyspark==4.2.0` under both ANSI settings: every integer width returns
the same values.
The narrow signature was chosen deliberately in #16125 from the
Databricks reference, which
documents the parameter as an INTEGER expression. That describes the
declared parameter type;
the accepted set is wider because of the implicit cast, which is only
visible by running Spark.
## What changes are included in this PR?
- `SparkFactorial` now uses `Signature::coercible` with an implicit
coercion from the Integer
type class to `Int32`, the same construct `round`, `width_bucket` and
`bit_get` in this crate
use. Planning inserts the cast, so `spark_factorial` still only receives
`Int32`.
- `factorial.slt`: replaces the assertion that `factorial(5::BIGINT)`
must fail with two blocks: one
aliased row covering the untyped literal, TINYINT, SMALLINT, BIGINT and
a bare NULL, and an
array-shaped BIGINT query covering a negative value, both range
boundaries, an above-range value
and NULL. Every expected value was observed from PySpark 4.2.0, not from
DataFusion output.
- Non-integer inputs still fail, now with DataFusion's standard coercion
error. Spark's wider
acceptance of STRING / DECIMAL / FLOAT inputs is tracked in #24941, and
the ANSI-dependent
handling of out-of-range integers in #24950 (part of #23929).
## What is the testing strategy for this PR?
The new `.slt` queries fail on `main` and pass with this change,
verified by checking out the
pre-fix implementation file and re-running `cargo test --test
sqllogictests -- spark/math/factorial`:
- with the old signature checked out: both blocks fail with
`coercion from Int64 to the signature Exact(Int32) failed`.
- restored: `Progress: 1/1 files completed (100%)`.
`cargo test -p datafusion-spark factorial`, `cargo fmt --all -- --check`
and
`cargo clippy -p datafusion-spark --all-targets -- -D warnings` pass.
The existing unit tests in
`factorial.rs` are unchanged.
## Are there any user-facing changes?
`factorial` in `datafusion-spark` now accepts all integer widths. No
breaking API change.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Fable 5.1 <no...@anthropic.com>
Co-authored-by: Oleks V <co...@users.noreply.github.com>
---
datafusion/spark/src/function/math/factorial.rs | 14 ++++++++++++--
.../sqllogictest/test_files/spark/math/factorial.slt | 19 +++++++++++++++++--
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/datafusion/spark/src/function/math/factorial.rs b/datafusion/spark/src/function/math/factorial.rs
index 3c41479174..5a54f2be97 100644
--- a/datafusion/spark/src/function/math/factorial.rs
+++ b/datafusion/spark/src/function/math/factorial.rs
@@ -21,11 +21,15 @@ use arrow::array::{Array, Int64Array};
use arrow::datatypes::DataType;
use arrow::datatypes::DataType::{Int32, Int64};
use datafusion_common::cast::as_int32_array;
+use datafusion_common::types::logical_int32;
use datafusion_common::{
DataFusionError, Result, ScalarValue, exec_err, utils::take_function_args,
};
use datafusion_expr::Signature;
-use datafusion_expr::{ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Volatility};
+use datafusion_expr::{
+ Coercion, ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, TypeSignatureClass,
+ Volatility,
+};
/// <https://spark.apache.org/docs/latest/api/sql/index.html#factorial>
#[derive(Debug, PartialEq, Eq, Hash)]
@@ -43,7 +47,13 @@ impl Default for SparkFactorial {
impl SparkFactorial {
pub fn new() -> Self {
Self {
- signature: Signature::exact(vec![Int32], Volatility::Immutable),
+ signature: Signature::coercible(
+ vec![Coercion::new_implicit_native(
+ logical_int32(),
+ vec![TypeSignatureClass::Integer],
+ )],
+ Volatility::Immutable,
+ ),
aliases: vec![],
}
}
diff --git a/datafusion/sqllogictest/test_files/spark/math/factorial.slt b/datafusion/sqllogictest/test_files/spark/math/factorial.slt
index f8eae5d95a..e39132c4c7 100644
--- a/datafusion/sqllogictest/test_files/spark/math/factorial.slt
+++ b/datafusion/sqllogictest/test_files/spark/math/factorial.slt
@@ -62,5 +62,20 @@ NULL
NULL
NULL
-query error Error during planning: Failed to coerce arguments to satisfy a call to 'factorial' function
-SELECT factorial(5::BIGINT);
+query IIIII
+SELECT factorial(5) AS i64_literal,
+ factorial(5::TINYINT) AS i8,
+ factorial(5::SMALLINT) AS i16,
+ factorial(5::BIGINT) AS i64,
+ factorial(NULL) AS null_input;
+----
+120 120 120 120 NULL
+
+query I
+SELECT factorial(a) FROM VALUES (-1::BIGINT), (0::BIGINT), (20::BIGINT), (21::BIGINT), (NULL) AS t(a);
+----
+NULL
+1
+2432902008176640000
+NULL
+NULL
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org