-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-55277][SQL] Add protobuf_funcs group for Protobuf SQL functions
#54059
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
Conversation
JIRA Issue Information=== Improvement SPARK-55277 === This comment was automatically generated by GitHub Actions |
allisonwang-db
left a comment
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.
Are there going to have more protobuf related functions besides these two?
|
Good question. I think there could be more for protobuf function comparing to others like json, csv, xml, etc., a clear group can give us more insight for what's missing for feature parity |
protobuf_funcs group for Protobuf SQL functions
dongjoon-hyun
left a comment
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.
+1, LGTM. Thank you for revisiting this area and re-organizing, @yaooqinn .
|
Thank you @dongjoon-hyun , merged to master |
c0c531c to
44d3554
Compare
|
looks like the gha gave me a wrong signal, let me revert and rerun the tests in PR |
Similar to how Avro functions have their own 'avro_funcs' group, Protobuf functions (from_protobuf, to_protobuf) should have their own 'protobuf_funcs' group instead of being grouped under 'misc_funcs'. This improves consistency with how other format-specific functions are organized and makes the documentation clearer for users. ### What changes were proposed in this pull request? - Move from_protobuf and to_protobuf from misc_funcs to protobuf_funcs - Add protobuf_funcs to the groups set in gen-sql-functions-docs.py ### Why are the changes needed? Consistency with avro_funcs, json_funcs, csv_funcs, xml_funcs groupings. ### Does this PR introduce _any_ user-facing change? No functional changes. The only difference is in how functions are grouped in documentation. ### How was this patch tested? Existing tests. ### Was this patch authored or co-authored using generative AI tooling? Yes, GitHub Copilot was used to assist with this change.
44d3554 to
f98168b
Compare
|
Merged to master |
What changes were proposed in this pull request?
Similar to how Avro functions have their own
avro_funcsgroup, Protobuf functions (from_protobuf,to_protobuf) should have their ownprotobuf_funcsgroup instead of being grouped undermisc_funcs.This improves consistency with how other format-specific functions are organized and makes the documentation clearer for users.
Changes:
from_protobufandto_protobuffrommisc_funcstoprotobuf_funcsprotobuf_funcsto the groups set ingen-sql-functions-docs.pyWhy are the changes needed?
Consistency with
avro_funcs,json_funcs,csv_funcs,xml_funcsgroupings.Does this PR introduce any user-facing change?
No functional changes. The only difference is in how functions are grouped in documentation.
How was this patch tested?
Existing tests.
Was this patch authored or co-authored using generative AI tooling?
Yes, GitHub Copilot was used to assist with this change.