Repository: databendlabs/databend (language: rust)

Issue:
bug: histogram function with bad argument cause panic
### Search before asking

- [X] I had searched in the [issues](https://github.com/databendlabs/databend/issues) and found no similar issues.


### Version

main

### What's Wrong?

found by duckdb fuzz test
```
panicked at src/query/expression/src/values.rs:453:18:
internal error: entered unreachable code: is_positive() called on non-numeric scalar
```

### How to Reproduce?

```sql
root@0.0.0.0:48000/default> CREATE TABLE obs (n Int64);
root@0.0.0.0:48000/default> SELECT histogram(n, [10, 20, 30, 40, 50]) FROM obs;
error: APIError: QueryFailed: [1104]internal error: entered unreachable code: is_positive() called on non-numeric scalar
```

### Are you willing to submit PR?

- [ ] Yes I am willing to submit a PR!


Discussion / hints:
@b41sh @sundy-li 
we do not support this kind of usage, just need to improve the error message?

https://docs.databend.com/sql/sql-functions/aggregate-functions/aggregate-histogram
@soyeric128  `HISTOGRAM(<expr> [, max_num_buckets])` is not supported, please correct the docs.

but we supported

```
HISTOGRAM(max_num_buckets)(<expr>)
```
@youngsofun  but  `assert_variadic_arguments` should be changed inside databend 
> [@b41sh](https://github.com/b41sh) [@sundy-li](https://github.com/sundy-li) we do not support this kind of usage, just need to improve the error message?
> 
> https://docs.databend.com/sql/sql-functions/aggregate-functions/aggregate-histogram

`is_positive` function should return an `Option`, and remove `unreachable` case. `histogram` should return an unsupported argumented error instead of panic.
> [@soyeric128](https://github.com/soyeric128) `HISTOGRAM(<expr> [, max_num_buckets])` is not supported, please correct the docs.
> 
> but we supported
> 
> ```
> HISTOGRAM(max_num_buckets)(<expr>)
> ```

Fixes: https://github.com/databendlabs/databend-docs/pull/1557

Fix patch:
diff --git a/src/query/expression/src/type_check.rs b/src/query/expression/src/type_check.rs
index 1e338cea0f9c3..3f11e208c565f 100755
--- a/src/query/expression/src/type_check.rs
+++ b/src/query/expression/src/type_check.rs
@@ -250,7 +250,11 @@ pub fn check_number<Index: ColumnIndex, T: Number>(
             ErrorCode::InvalidArgument(format!("Expect {}, but got {}", T::data_type(), origin_ty))
                 .set_span(span)
         }),
-        _ => Err(ErrorCode::InvalidArgument("Need constant number").set_span(span)),
+        _ => Err(ErrorCode::InvalidArgument(format!(
+            "Need constant number, but got {}",
+            expr.sql_display()
+        ))
+        .set_span(span)),
     }
 }
 
diff --git a/src/query/sql/src/planner/semantic/type_check.rs b/src/query/sql/src/planner/semantic/type_check.rs
index df2eac94a1ced..02789950c9939 100644
--- a/src/query/sql/src/planner/semantic/type_check.rs
+++ b/src/query/sql/src/planner/semantic/type_check.rs
@@ -1729,19 +1729,14 @@ impl<'a> TypeChecker<'a> {
             && arguments.len() == 2
             && params.is_empty()
         {
-            let max_num_buckets = ConstantExpr::try_from(arguments[1].clone());
-
-            let is_positive_integer = match &max_num_buckets {
-                Ok(v) => v.value.is_positive(),
-                Err(_) => false,
-            } && arg_types[1].is_integer();
-            if !is_positive_integer {
-                return Err(ErrorCode::SemanticError(
-                    "The max_num_buckets of `histogram` must be a constant positive int",
-                ));
-            }
+            let max_num_buckets: u64 = check_number(
+                None,
+                &FunctionContext::default(),
+                &arguments[1].as_expr()?,
+                &BUILTIN_FUNCTIONS,
+            )?;
 
-            vec![max_num_buckets.unwrap().value]
+            vec![Scalar::Number(NumberScalar::UInt64(max_num_buckets))]
         } else {
             params
         };


Test patch:
diff --git a/tests/sqllogictests/suites/query/functions/02_0000_function_aggregate_histogram.test b/tests/sqllogictests/suites/query/functions/02_0000_function_aggregate_histogram.test
index 3f4316ada19b1..a276cc3fb6666 100644
--- a/tests/sqllogictests/suites/query/functions/02_0000_function_aggregate_histogram.test
+++ b/tests/sqllogictests/suites/query/functions/02_0000_function_aggregate_histogram.test
@@ -1,5 +1,18 @@
+statement error 2004
+SELECT histogram(n, [10, 20, 30, 40, 50]) FROM numbers(100) t(n);
+
+query T
+SELECT histogram(n, 1) FROM numbers(100) t(n);
+----
+[{"lower":"0","upper":"99","ndv":100,"count":100,"pre_sum":0}]
+
+query T
+SELECT histogram(3)(n) FROM numbers(100) t(n);
+----
+[{"lower":"0","upper":"33","ndv":34,"count":34,"pre_sum":0},{"lower":"34","upper":"67","ndv":34,"count":34,"pre_sum":34},{"lower":"68","upper":"99","ndv":32,"count":32,"pre_sum":68}]
+
 statement ok
-create table histagg (
+create or replace table histagg (
     c_id INT,
     c_tinyint TINYINT,
     c_smallint SMALLINT,
@@ -18,42 +31,42 @@ create table histagg (
 
 statement ok
 INSERT INTO histagg values
-    (1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
+    (1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
     NULL, NULL, 'not null'),
     (1, 10, 20, 30, 4444444444444, 0.1, 0.222, 3333.33, 'c', 'varchar1', 'string1',
     '2022-12-01', '2022-12-01 22:23:23', 'not null');
 
 statement ok
 INSERT INTO histagg values
-    (1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
+    (1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
     NULL, NULL, 'not null'),
     (1, 11, 21, 33, 4444444444444, 0.1, 0.222, 3333.33, 'c', 'varchar1', 'string1',
     '2022-12-01', '2022-12-01 22:23:23', 'not null');
 
 statement ok
 INSERT INTO histagg values
-    (1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
+    (1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
     NULL, NULL, 'not null'),
     (1, 11, 12, 13, 1444444444444, 1.1, 1.222, 13333.33, 'd', 'varchar2', 'string2',
     '2022-12-02', '2022-12-02 22:23:23', 'not null');
 
 statement ok
 INSERT INTO histagg values
-    (2, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
+    (2, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
     NULL, NULL, 'not null'),
     (2, 21, 22, 23, 2444444444444, 2.1, 2.222, 23333.33, 'f', 'varchar3', 'string3',
     '2022-12-03', '2022-12-03 22:23:23', 'not null');
 
 statement ok
 INSERT INTO histagg values
-    (2, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
+    (2, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
     NULL, NULL, 'not null'),
     (2, 31, 32, 33, 3444444444444, 3.1, 3.222, 33333.33, 'l', 'varchar3', 'string3',
     '2022-12-03', '2022-12-03 22:23:23', 'not null');
 
 statement ok
 INSERT INTO histagg values
-    (2, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
+    (2, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
     NULL, NULL, 'not null'),
     (2, 10, 20, 30, 944444444444, 9.1, 9.222, 93333.33, 'p', 'varchar9', 'string9',
     '2022-12-09', '2022-12-09 22:23:23', 'not null');
@@ -139,7 +152,7 @@ select histogram() from histagg;
 statement error
 select histogram(c_id, "1") from histagg;
 
-statement error
+statement ok
 select histogram(c_id, 1.1) from histagg;
 
 statement error
diff --git a/tests/sqllogictests/suites/query/functions/02_0000_function_aggregate_mix.test b/tests/sqllogictests/suites/query/functions/02_0000_function_aggregate_mix.test
index e07ee66404f7d..0f3ba84373bbc 100644
--- a/tests/sqllogictests/suites/query/functions/02_0000_function_aggregate_mix.test
+++ b/tests/sqllogictests/suites/query/functions/02_0000_function_aggregate_mix.test
@@ -345,7 +345,7 @@ select group_array_moving_avg(k), group_array_moving_avg(2)(v) from aggr;
 ----
 [0.09090909090909091,0.2727272727272727,0.45454545454545453,0.6363636363636364,0.8181818181818182,1.0,1.1818181818181819,1.3636363636363635,1.5454545454545454,1.7272727272727273,1.9090909090909092] [5.0,10.0,10.0,10.0,15.0,20.0,22.5,27.5,30.0,30.0,30.0]
 
-statement error Need constant number
+statement error 2004
 SELECT group_array_moving_sum('x')(-1130932975.87767);
 
 query TTT