alamb commented on code in PR #9782:
URL: https://github.com/apache/arrow-datafusion/pull/9782#discussion_r1536875793
##########
datafusion/sqllogictest/test_files/expr.slt:
##########
@@ -415,6 +415,12 @@ SELECT chr(CAST(NULL AS int))
----
NULL
+statement error DataFusion error: Execution error: null character not
permitted.
Review Comment:
👍
##########
datafusion/core/tests/dataframe/dataframe_functions.rs:
##########
@@ -111,7 +111,7 @@ async fn test_fn_ascii() -> Result<()> {
#[tokio::test]
async fn test_fn_bit_length() -> Result<()> {
- let expr = bit_length(col("a"));
+ let expr = bit_length(vec![col("a")]);
Review Comment:
since `bit_length` only ever takes a single parameter, I think it would be
better to keep this as a single `expr` argument (to keep the UX nicer to call
`bit_length`).
If the function can take potentially different numbers of parameters, that
is when `Vec<Expr>` is more appropriate I think
##########
datafusion/functions/src/string/mod.rs:
##########
@@ -57,11 +61,21 @@ pub mod expr_fn {
super::ascii().call(vec![arg1])
}
+ #[doc = "Returns the number of bits in the `string`"]
+ pub fn bit_length(args: Vec<Expr>) -> Expr {
+ super::bit_length().call(args)
+ }
+
#[doc = "Removes all characters, spaces by default, from both sides of a
string"]
pub fn btrim(args: Vec<Expr>) -> Expr {
super::btrim().call(args)
}
+ #[doc = "Converts the Unicode code point to a UTF8 character"]
+ pub fn chr(args: Vec<Expr>) -> Expr {
Review Comment:
Likewise here, I think this should take a single `Expr`
##########
datafusion/functions/src/string/mod.rs:
##########
@@ -57,11 +61,21 @@ pub mod expr_fn {
super::ascii().call(vec![arg1])
}
+ #[doc = "Returns the number of bits in the `string`"]
+ pub fn bit_length(args: Vec<Expr>) -> Expr {
Review Comment:
See above -- I think this should take a single `Expr` argument rather than a
`Vec<Expr>`
--
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]
