move Left, Lpad, Reverse, Right, Rpad functions to datafusion_functions #9841
Conversation
…ailable feature in DataFusion and building with nightly may not be a good recommendation when getting started.
…_expressions feature flag, move char_length function
# Conflicts: # datafusion/expr/src/built_in_function.rs # datafusion/functions/src/unicode/mod.rs # datafusion/physical-expr/src/unicode_expressions.rs
github-actions
bot
added
logical-expr
|
|
||
| #[test] | ||
| fn test_functions() -> Result<()> { | ||
| #[cfg(feature = "unicode_expressions")] |
There was a problem hiding this comment.
Since the whole module is cfg'd we can probably remove these guards on individual tests
| ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("abcde")))), | ||
| ColumnarValue::Scalar(ScalarValue::Int64(Some(2))), |
There was a problem hiding this comment.
Totally not needed as this code is just moved, but I think you can write this more concisely with from if you wnat:
| ColumnarValue::Scalar(ScalarValue::Utf8(Some(String::from("abcde")))), | |
| ColumnarValue::Scalar(ScalarValue::Int64(Some(2))), | |
| ColumnarValue::Scalar(ScalarValue::from("abcde")), | |
| ColumnarValue::Scalar(ScalarValue::from(2u64)), |
There was a problem hiding this comment.
(same thing applies to the rest of the tests in this file and and the others)
| use crate::utils::test::test_function; | ||
|
|
||
| #[test] | ||
| fn test_functions() -> Result<()> { |
There was a problem hiding this comment.
(same comments as above about cfg and ScalarValue::from apply here too)
|
Thanks for the feedback - I'd like to update this PR with the feedback here rather than in a followup commit if you don't mind. |
|
👨🍳 👌 -- looks really nice. Thanks for cleaning up the code @Omega359 |
|
It is like the scouts "leave things better than how you found them" |
…ns (apache#9841) * Fix to_timestamp benchmark * Remove reference to simd and nightly build as simd is no longer an available feature in DataFusion and building with nightly may not be a good recommendation when getting started. * Fixed missing trim() function. * Create unicode module in datafusion/functions/src/unicode and unicode_expressions feature flag, move char_length function * move Left, Lpad, Reverse, Right, Rpad functions to datafusion_functions * Code cleanup from PR review.
Which issue does this PR close?
Closes #9828
Rationale for this change
As part of #9285 the unicode functions should be migrated to the new datafusion-functions crate in the new structure
What changes are included in this PR?
Code, tests.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.