alamb commented on code in PR #5807:
URL: https://github.com/apache/arrow-datafusion/pull/5807#discussion_r1154694292
##########
datafusion/physical-expr/src/math_expressions.rs:
##########
@@ -149,6 +160,58 @@ pub fn random(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
Ok(ColumnarValue::Array(Arc::new(array)))
}
+/// Round SQL function
+pub fn round(args: &[ArrayRef]) -> Result<ArrayRef> {
+ if args.len() != 1 && args.len() != 2 {
+ return Err(DataFusionError::Internal(
+ "round function requires one or two arguments".to_string(),
Review Comment:
```suggestion
format!("round function requires one or two arguments, got {}",
args.len())
```
##########
datafusion/physical-expr/src/math_expressions.rs:
##########
@@ -365,4 +428,40 @@ mod tests {
assert_eq!(floats.value(2), 4.0);
assert_eq!(floats.value(3), 4.0);
}
+
+ #[test]
+ fn test_round_f32() {
+ let args: Vec<ArrayRef> = vec![
+ Arc::new(Float32Array::from(vec![125.2345; 10])), // input
+ Arc::new(Int64Array::from(vec![0, 1, 2, 3, 4, 5, -1, -2, -3,
-4])), // decimal_places
+ ];
+
+ let result = round(&args).expect("failed to initialize function
round");
+ let floats =
+ as_float32_array(&result).expect("failed to initialize function
round");
+
+ let expected = Float32Array::from(vec![
+ 125.0, 125.2, 125.23, 125.235, 125.2345, 125.2345, 130.0, 100.0,
0.0, 0.0,
Review Comment:
I spot checked that these answers are consistent with postgres 👍 (including
negative)
```sql
postgres=# select round(125.2345, -1);
round
-------
130
(1 row)
postgres=# select round(125.2345, -2);
round
-------
100
(1 row)
postgres=# select round(125.2345, 3);
round
---------
125.235
(1 row)
```
##########
datafusion/physical-expr/src/math_expressions.rs:
##########
@@ -149,6 +160,58 @@ pub fn random(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
Ok(ColumnarValue::Array(Arc::new(array)))
}
+/// Round SQL function
+pub fn round(args: &[ArrayRef]) -> Result<ArrayRef> {
+ if args.len() != 1 && args.len() != 2 {
+ return Err(DataFusionError::Internal(
+ "round function requires one or two arguments".to_string(),
+ ));
+ }
+
+ let mut decimal_places =
+ &(Arc::new(Int64Array::from_value(0, args[0].len())) as ArrayRef);
Review Comment:
It might be more efficient to make a ColumnarValue to avoid creating a large
empty array. That would complicate the implementation as it would have to
handle both the array argument form and the scalar argument form
--
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]
