2010YOUY01 commented on code in PR #8140:
URL: https://github.com/apache/arrow-datafusion/pull/8140#discussion_r1390610500


##########
docs/source/user-guide/sql/scalar_functions.md:
##########
@@ -2426,6 +2427,20 @@ trim_array(array, n)
   Can be a constant, column, or function, and any combination of array 
operators.
 - **n**: Element to trim the array.
 
+### `range`

Review Comment:
   This is clear and precise 💯 



##########
datafusion/physical-expr/src/array_expressions.rs:
##########
@@ -579,6 +579,41 @@ pub fn array_pop_back(args: &[ArrayRef]) -> 
Result<ArrayRef> {
     )
 }
 
+pub fn gen_range(args: &[ArrayRef]) -> Result<ArrayRef> {
+    let (start_array, stop_array, step_array) = match args.len() {
+        1 => (None, as_int64_array(&args[0])?, None),
+        2 => (
+            Some(as_int64_array(&args[0])?),
+            as_int64_array(&args[1])?,
+            None,
+        ),
+        3 => (
+            Some(as_int64_array(&args[0])?),
+            as_int64_array(&args[1])?,
+            Some(as_int64_array(&args[2])?),
+        ),
+        _ => return internal_err!("gen_range expects 1 to 3 arguments"),
+    };
+
+    let mut values = vec![];
+    let mut offsets = vec![0];
+    for (idx, stop) in stop_array.iter().enumerate() {
+        let stop = stop.unwrap_or(0);
+        let start = start_array.as_ref().map(|arr| 
arr.value(idx)).unwrap_or(0);
+        let step = step_array.as_ref().map(|arr| arr.value(idx)).unwrap_or(1);

Review Comment:
   We might not want the bad input to crash at run time
   ```
   ❯ select range(1,5,0);
   thread 'main' panicked at 'assertion failed: step != 0', 
/rustc/e6d4725c76f3b526c74454bc51afdf6daf133506/library/core/src/iter/adapters/step_by.rs:21:9
   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
   ```
   Maybe `if step == 0 { return exec_err!("step can't be 0 for function 
range(start [, stop, step]")}`?



##########
datafusion/physical-expr/src/array_expressions.rs:
##########
@@ -579,6 +579,41 @@ pub fn array_pop_back(args: &[ArrayRef]) -> 
Result<ArrayRef> {
     )
 }
 
+pub fn gen_range(args: &[ArrayRef]) -> Result<ArrayRef> {
+    let (start_array, stop_array, step_array) = match args.len() {
+        1 => (None, as_int64_array(&args[0])?, None),
+        2 => (
+            Some(as_int64_array(&args[0])?),
+            as_int64_array(&args[1])?,
+            None,
+        ),
+        3 => (
+            Some(as_int64_array(&args[0])?),
+            as_int64_array(&args[1])?,
+            Some(as_int64_array(&args[2])?),
+        ),
+        _ => return internal_err!("gen_range expects 1 to 3 arguments"),
+    };
+
+    let mut values = vec![];
+    let mut offsets = vec![0];
+    for (idx, stop) in stop_array.iter().enumerate() {
+        let stop = stop.unwrap_or(0);
+        let start = start_array.as_ref().map(|arr| 
arr.value(idx)).unwrap_or(0);
+        let step = step_array.as_ref().map(|arr| arr.value(idx)).unwrap_or(1);

Review Comment:
   We might not want the bad input to crash at run time
   ```
   ❯ select range(1,5,0);
   thread 'main' panicked at 'assertion failed: step != 0', 
/rustc/e6d4725c76f3b526c74454bc51afdf6daf133506/library/core/src/iter/adapters/step_by.rs:21:9
   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
   ```
   Maybe `if step == 0 { return exec_err!("step can't be 0 for function 
range(start [, stop, step]")}`?



##########
docs/source/user-guide/sql/scalar_functions.md:
##########
@@ -2426,6 +2427,20 @@ trim_array(array, n)
   Can be a constant, column, or function, and any combination of array 
operators.
 - **n**: Element to trim the array.
 
+### `range`

Review Comment:
   This is clear and precise 💯 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to