fix(query): reject negative factorial inputs (#19891)

Author: KKould

src/query/functions/src/scalars/mathematics/src/math.rs

@@ -452,10 +452,20 @@ pub fn register(registry: &mut FunctionRegistry) {
                     |_, _| FunctionDomain::MayThrow,
                     vectorize_with_builder_1_arg::<NumberType<NUM_TYPE>, NumberType<i64>>(
                         |val, output, ctx| {
-                            let n: i64 = AsPrimitive::<i64>::as_(val);
-                            if n > MAX_FACTORIAL_NUMBER {
+                            let max = AsPrimitive::<NUM_TYPE>::as_(MAX_FACTORIAL_NUMBER);
+                            if val > max {
                                 ctx.set_error(output.len(), format!("factorial number is out of range, max is: {}", MAX_FACTORIAL_NUMBER));
                                 output.push(0);
+                                return;
+                            }
+
+                            let n = AsPrimitive::<i64>::as_(val);
+                            if n < 0 {
+                                ctx.set_error(
+                                    output.len(),
+                                    "factorial number is out of range, min is: 0".to_string(),
+                                );
+                                output.push(0);
                             } else {
                                 output.push(factorial(n));
                             }
@@ -567,5 +577,5 @@ type Log10Function = GenericLogFunction<TenBase>;
 type Log2Function = GenericLogFunction<TwoBase>;
 
 fn factorial(n: i64) -> i64 {
-    if n <= 0 { 1 } else { n * factorial(n - 1) }
+    if n == 0 { 1 } else { n * factorial(n - 1) }
 }

src/query/functions/tests/it/scalars/math.rs

@@ -141,7 +141,12 @@ fn test_cbrt(file: &mut impl Write) {
 
 fn test_factorial(file: &mut impl Write) {
     run_ast(file, "factorial(5)", &[]);
+    run_ast(file, "factorial(-1)", &[]);
     run_ast(file, "factorial(30)", &[]);
+    run_ast(file, "factorial(u)", &[(
+        "u",
+        UInt64Type::from_data(vec![u64::MAX]),
+    )]);
     run_ast(file, "factorial(a)", &[(
         "a",
         Int64Type::from_data(vec![3, 12, 16]),

src/query/functions/tests/it/scalars/testdata/math.txt

@@ -636,6 +636,14 @@ output domain  : {120..=120}
 output         : 120
 
 
+error: 
+  --> SQL:1:1
+  |
+1 | factorial(-1)
+  | ^^^^^^^^^^^^^ factorial number is out of range, min is: 0 while evaluating function `factorial(-1)` in expr `factorial(-1)`
+
+
+
 error: 
   --> SQL:1:1
   |
@@ -644,6 +652,14 @@ error:
 
 
 
+error: 
+  --> SQL:1:1
+  |
+1 | factorial(u)
+  | ^^^^^^^^^^^^ factorial number is out of range, max is: 20 while evaluating function `factorial(18446744073709551615)` in expr `factorial(u)`
+
+
+
 ast            : factorial(a)
 raw expr       : factorial(a::Int64)
 checked expr   : factorial<Int64>(a)

tests/sqllogictests/suites/query/functions/02_0014_function_maths.test

@@ -329,10 +329,18 @@ SELECT factorial(5)
 ----
 120
 
+statement error 1006
+SELECT factorial(-1)
+
+
 statement error 1006
 SELECT factorial(30)
 
 
+statement error 1006
+SELECT factorial(18446744073709551615::UInt64)
+
+
 statement error 1006
 SELECT pow('a', 2)