feat: Migrate Map Functions #13047
feat: Migrate Map Functions #13047
Conversation
github-actions
bot
added
documentation
|
This is tested with the map function, @Omega359 I am open to any changes you want to make here. |
datafusion/expr/src/udsf.rs
Outdated
|
|
||
| //! [`SpecialUDF`]: Special User Defined Functions | ||
|
|
||
| pub mod special_doc_sections { |
There was a problem hiding this comment.
It seems like map is still a scalar function -- I was thinking that the DocSection was what was special (as in it would be DOC_SECTION_SPECIAL that was part of scalar_doc_sections 🤔
There was a problem hiding this comment.
Yeah, I think unnest truly is special (as it changes the schema of the output) and has its own Expr variant: https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.Expr.html#variant.Unnest
Maybe we can just have static docs for that one
There was a problem hiding this comment.
If it is indeed just one or two functions we could just have them in the update script or in a file that is pulled into the final .md file by that script?
There was a problem hiding this comment.
Yeah that sounds fine, it seems that unnest is one of the very few special functions. But i feel that it would be easier for them to just be static.
| fn get_map_doc() -> &'static Documentation { | ||
| DOCUMENTATION.get_or_init(|| { | ||
| Documentation::builder() | ||
| .with_doc_section(DOC_SECTION_SPECIAL) |
There was a problem hiding this comment.
I think I am missing something -- it seems like the map function should be in a "Map Functions" section to follow the existing documentation:
https://datafusion.apache.org/user-guide/sql/scalar_functions.html#map-functions
There was a problem hiding this comment.
It seems like
mapis still a scalar function -- I was thinking that theDocSectionwas what was special (as in it would beDOC_SECTION_SPECIALthat was part ofscalar_doc_sections🤔
I think you mentioned above that you wanted the function to put in a doc_section_special here. So should I make the map function go under 'doc section map' instead?
There was a problem hiding this comment.
I would say so, yes. My original thought was that unnest and make_map would be the two special functions but digging through the planner code it seems that make_map just calls into map udf (#11434) so I think the only special one is unnest.
There was a problem hiding this comment.
Alright, so the direction of this pr now is to just add a static page with unnest?
There was a problem hiding this comment.
Sorry for the confusion -- I think there are two distinct action items:
- Migrate the documentation for
mapand related functions to a "Map Functions" section - Create a new static page for "special functions" (different than Scalar Functions) where we can put
unnest
Perhaps we can turn this PR into the map and related functions in a "Map Functions" section and make another PR to move the unnest docs to the new page.
I am happy to help with either
Thanks again for your help @jonathanc-n and @Omega359
jonathanc-n
changed the title
|
I migrated the map functions, however it should be noted that i had combined make_map and map since they were under the same udf. Is there a better way to do this? |
No -- I think this is the best way actually as they really are the same they should be listed as aliases |
There was a problem hiding this comment.
Thank you @jonathanc-n -- this looks great to me now
Thank you for bearing with us
|
🚀 lets keep things moving |
Which issue does this PR close?
Closes #13021 .
Rationale for this change
What changes are included in this PR?
Migrate Map Functions
Are these changes tested?
Are there any user-facing changes?