Skip to content

Commit 2d1d53e

Browse files
raushanprabhakar1Raushan PrabhakarkumarUjjawal
authored
fixing factorial negative values (#22278)
## Which issue does this PR close? - Closes #22270 ## Rationale for this change PostgreSQL returns a domain error for `factorial` on negative inputs (`ERROR: factorial of a negative number is undefined`). DataFusion incorrectly returned `1` for negative values (e.g. `factorial(-1)`), which breaks PostgreSQL-aligned SQL semantics and can silently produce wrong results. ## What changes are included in this PR? - **`factorial` (core / `datafusion-functions`):** Negative arguments now fail with an execution error and message aligned with PostgreSQL: `factorial of a negative number is undefined`, instead of returning `1`. - **Documentation:** Updated the `#[user_doc]` description so it matches behavior (non-negative inputs; error on negative values and on overflow). - **Tests:** Added a sqllogictest in `math.slt` that expects this error for `factorial(-1)`. **Note:** Spark’s `factorial` in `datafusion-spark` is unchanged (PySpark-style nulls for out-of-domain values). ## Are these changes tested? Yes. A sqllogictest was added in `datafusion/sqllogictest/test_files/math.slt` for `SELECT factorial(-1)`. ## Are there any user-facing changes? Yes. `factorial` with a negative argument now errors at execution time instead of returning `1`. This matches PostgreSQL and fixes incorrect results; callers that relied on the old behavior would need to adjust. No public Rust API breakage in exported types; behavior change is SQL/UDF semantics for negative inputs. --------- Co-authored-by: Raushan Prabhakar <ros@Raushans-MacBook-Air.local> Co-authored-by: Kumar Ujjawal <ujjawalpathak6@gmail.com>
1 parent 6e58d74 commit 2d1d53e

3 files changed

Lines changed: 7 additions & 3 deletions

File tree

datafusion/functions/src/math/factorial.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use datafusion_macros::user_doc;
3232

3333
#[user_doc(
3434
doc_section(label = "Math Functions"),
35-
description = "Factorial. Returns 1 if value is less than 2.",
35+
description = "Factorial of a non-negative integer. Errors if the argument is negative or the result overflows.",
3636
syntax_example = "factorial(numeric_expression)",
3737
sql_example = r#"```sql
3838
> SELECT factorial(5);
@@ -143,7 +143,7 @@ const FACTORIALS: [i64; 21] = [
143143

144144
fn compute_factorial(n: i64) -> Result<i64> {
145145
if n < 0 {
146-
Ok(1)
146+
exec_err!("factorial of a negative number is undefined")
147147
} else if n < FACTORIALS.len() as i64 {
148148
Ok(FACTORIALS[n as usize])
149149
} else {

datafusion/sqllogictest/test_files/math.slt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,10 @@ select
826826
----
827827
39 Decimal128(2, 1)
828828

829+
# factorial negative (PostgreSQL-compatible domain error)
830+
query error DataFusion error: Execution error: factorial of a negative number is undefined
831+
select factorial(-1);
832+
829833
# factorial overflow
830834
query error DataFusion error: Execution error: Overflow happened on FACTORIAL\(350943270\)
831835
select FACTORIAL(350943270);

docs/source/user-guide/sql/scalar_functions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ exp(numeric_expression)
420420

421421
### `factorial`
422422

423-
Factorial. Returns 1 if value is less than 2.
423+
Factorial of a non-negative integer. Errors if the argument is negative or the result overflows.
424424

425425
```sql
426426
factorial(numeric_expression)

0 commit comments

Comments
 (0)