You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/02/01 12:05:26 UTC
[I] [DISCUSS] organization of functions [arrow-datafusion]
alamb opened a new issue, #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100
### Is your feature request related to a problem or challenge?
As pointed out by @viirya on https://github.com/apache/arrow-datafusion/pull/8705#discussion_r1470110480 there are many potential ways to organize the data fusion function library as we break it out of the core in https://github.com/apache/arrow-datafusion/issues/8045
I would like to get some consensus on how we want the organization to look before creating tickets and starting to crank it out
### Describe the solution you'd like
Here is a proposal for how the functions are organized.
## math functions
* feature_flag (new): `math_expressions`
* code location: `datafusion/functions/src/math`
* Abs, Acos, Asin, Atan, Atan2, Acosh, Asinh, Atanh, Cbrt, Ceil, Cos, Cosh, Degrees, Exp, Factorial, Floor, Gcd, Lcm, Ln, Log, Log10, Log2, Pi, Power, Radians, Signum, Sin, Sinh, Sqrt, Tan, Tanh, Trunc, Cot, Round, iszero
## array functions
**Given the size and specialization of these functions I propose putting them into their own subcrate**
* feature_flag (new): `array_expressions`
* code location: `datafusion/functions-array/src/math`
* ArrayAppend, ArraySort, ArrayConcat, ArrayHas, ArrayHasAll, ArrayHasAny, ArrayPopFront, ArrayPopBack, ArrayDims, ArrayDistinct, ArrayElement, ArrayEmpty, ArrayLength, ArrayNdims, ArrayPosition, ArrayPositions, ArrayPrepend, ArrayRemove, ArrayRemoveN, ArrayRemoveAll, ArrayRepeat, ArrayReplace, ArrayReplaceN, ArrayReplaceAll, ArraySlice, ArrayToString, ArrayIntersect, ArrayUnion, ArrayExcept, Cardinality, ArrayResize, Flatten, Range, StringToArray,
## Core functions
These functions are always available as they are used for internal purposes (like implementing `[1,2,3]` syntax in SQL or so commonly used that it is not worth having a feature flag)
* feature_flag: NONE
* code location: `datafusion/functions/src/core` or similar
* `MakeArray`: construct an array from columns
* `Isnan`: is the value NaN
* `Coalesce`: return the first non-null value
* `Nanvl`: return the first non-NaN value
* `Struct`: Create a struct
* `NullIf`: return null if the two values are equal
* `Random`: return a random number
* `ArrowTypeOf`: return the arrow type of a value
## Crypto functions
* feature_flag (existing): `crypto_expressions`
* code location: `datafusion/functions/src/crypto`
* Digest, MD5, SHA224, SHA256, SHA384, SHA512
## String functions
* feature_flag (new): `string_expressions`
* code location: `datafusion/functions/src/string`
* ascii, bit_length, btrim, chr, concat, concat_ws, ends_with, initcap, instr, lower, ltrim, octet_length, repeat, replace, rtrim, split_part, starts_with, to_hex, trim, upper, levenshtein, uuid, overlay
## Unicode string functions
These expressions need an additioanl dependency, which is why they have a different flag)
* feature_flag (existing): `unicode_expressions`
* code location: `datafusion/functions/src/string/unicode`
* CharLength, Left, Lpad, Reverse, Right, Rpad, Strpos, Substr, Translate, SubstrIndex, FindInSet
## regex functions
* feature_flag (existing): `regex_expressions`
* code location: `datafusion/functions/src/regexp`
* RegexpMatch, RegexpReplace
## date time function
* feature_flag (new): `datetime_expressions`
* code location: `datafusion/functions/src/datetime`
* date_part, date_trunc, date_bin, to_timestamp, to_timestamp_millis, to_timestamp_micros, to_timestamp_nanos, to_timestamp_seconds, from_unixtime, now, current_date, current_time
### Describe alternatives you've considered
We can have more fine grained crates, or different organizations, etc
For example, perhaps we should pull the string functions into `datafusion/functions-string` crate
### Additional context
_No response_
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1939421178
For anyone following along, we just merged https://github.com/apache/arrow-datafusion/pull/9113 to break out the array functions crate
What I plan to do this week is to make a few other PRs moving some functions so we have some more examples, and then file a bunch of "help wanted / good first issue tickets" to port the remaining functions per the list on this ticket
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #9100: [DISCUSS] organization of functions
URL: https://github.com/apache/arrow-datafusion/issues/9100
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1924155981
I whipped up an example of how an `datafusion-functions-array` crate might look: https://github.com/apache/arrow-datafusion/pull/9113
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1921257160
I think it is a good idea to move array functions into their crate given the size of it. It will become more complex since there are still tons of array functions not yet implemented. For other functions like string, if they are large enough, we should move them into subcrate as well. I think we should keep the core as simple as possible.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "Omega359 (via GitHub)" <gi...@apache.org>.
Omega359 commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1924205410
Minor: can we match the feature flag prefix with the source code location? Just seems cleaner to me.
string_expressions
code location: datafusion/functions/src/string
unicode_expressions
code location: datafusion/functions/src/unicode
regexp_expressions
code location: datafusion/functions/src/regexp
.. etc
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1953658408
I filed https://github.com/apache/arrow-datafusion/issues/9285 to track the actual work, let's continue the discussion there
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1921180990
FYI @Weijun-H and @jayzhan211 -- do you have any thoughts about moving the array functions into their own crate?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "l1t1 (via GitHub)" <gi...@apache.org>.
l1t1 commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1959397818
please add table functions, eg. generate_series
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1964844184
> please add table functions, eg. generate_series
Seems like this request was made in https://github.com/apache/arrow-datafusion/issues/9323 ✅ -- thank you
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "Weijun-H (via GitHub)" <gi...@apache.org>.
Weijun-H commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1921368831
It is timely to address the issues with array_experssion.rs, as @jayzhan211 stated that a lot of work still needs to be done on the array. I strongly support this ticket.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1933850627
@Omega359 asked a good question on https://github.com/apache/arrow-datafusion/pull/8705#issuecomment-1932661075 that I wanted to not get lost in comments on a merged pull request
> * Where should public rustdocs for the function go?
I think they should go in the `datafusion-functions` crate which is then both re-exported by the main datafusion crate:
<img width="1064" alt="Screenshot 2024-02-08 at 6 05 00 AM" src="https://github.com/apache/arrow-datafusion/assets/490673/80a4e392-ee73-409a-a386-2abe38bdcc57">
Following the model of the existing `expr_fn`s the `encode` and `decode` functions were re-exported in `datafusion::prelude` as well:
<img width="1078" alt="Screenshot 2024-02-08 at 6 06 33 AM" src="https://github.com/apache/arrow-datafusion/assets/490673/814cdf02-a48b-4b30-bfad-482cfb3d48d4">
> * Benchmarks? Tests && test data? Is that all going to live at functions/benches and functions/tests ?
I think we should do this if/when possible -- benchmarks that depend on `SessionContext` for example probably need to stay in datafusion/core but otherwise we should move them to datafusion-functions
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [DISCUSS] organization of functions [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #9100:
URL: https://github.com/apache/arrow-datafusion/issues/9100#issuecomment-1941385424
Note for myself: I tried to pull MakeArray into `datafusion-functions` core, but I found several other array functions (like union) rely on it, so I think it needs to go into `datafusion-functions-array`
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org