-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
move the Translate, SubstrIndex, FindInSet functions to datafusion-functions #9864
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
…n-functions crate
# Conflicts: # datafusion/expr/src/built_in_function.rs # datafusion/expr/src/expr_fn.rs # datafusion/functions/src/unicode/left.rs # datafusion/functions/src/unicode/lpad.rs # datafusion/functions/src/unicode/mod.rs # datafusion/functions/src/unicode/reverse.rs # datafusion/functions/src/unicode/right.rs # datafusion/functions/src/unicode/rpad.rs # datafusion/functions/src/unicode/substr.rs # datafusion/physical-expr/src/functions.rs # datafusion/physical-expr/src/unicode_expressions.rs # datafusion/proto/src/generated/pbjson.rs # datafusion/proto/src/generated/prost.rs # datafusion/proto/src/logical_plan/from_proto.rs # datafusion/proto/src/logical_plan/to_proto.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @Omega359 -- this looks great. I merged up from main to resolve a conflict
@@ -67,8 +67,6 @@ regex_expressions = [ | |||
] | |||
serde = ["arrow-schema/serde"] | |||
unicode_expressions = [ | |||
"datafusion-physical-expr/unicode_expressions", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
return Some(String::new()); | ||
} | ||
|
||
let splitted: Box<dyn Iterator<Item = _>> = if n > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can probably make this significantly faster by avoiding the Box::new()
here and the call to to_owned()
below. I realize this is not something changed in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll create a separate benchmark for this.
Thanks again @Omega359 |
…nctions (apache#9864) * 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 * move strpos, substr functions to datafusion_functions * move the Translate, SubstrIndex, FindInSet functions to new datafusion-functions crate * Test code cleanup * unicode_expressions Cargo.toml updates. --------- Co-authored-by: Andrew Lamb <[email protected]>
Which issue does this PR close?
Closes #9842
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,