-
Notifications
You must be signed in to change notification settings - Fork 232
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
Auto-register UDF extention when main plugin is set #1102
Conversation
Signed-off-by: Allen Xu <allxu@nvidia.com>
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.
Looks good to me
build |
We should also update documentation:
Could happen in a different PR IMHO. |
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.
Could we remove the extensions modification from the UDF tests?
.set("spark.sql.extensions", "com.nvidia.spark.udf.Plugin") |
Removing current extension requires adding main sql plugin config. But that will fail the OpcodeSuite. |
I would like to see the documentation updated as a part of this PR, just so it is not forgotten. |
Signed-off-by: Allen Xu <allxu@nvidia.com>
docs/compatibility.md
Outdated
@@ -290,10 +290,10 @@ Casting from string to timestamp currently has the following limitations. | |||
Only timezone 'Z' (UTC) is supported. Casting unsupported formats will result in null values. | |||
|
|||
## UDF to Catalyst Expressions | |||
To speedup the process of UDF, spark-rapids introduces a udf-compiler extension to translate UDFs to Catalyst expressions. | |||
To speedup the process of UDF, spark-rapids introduces a udf-compiler extension to translate UDFs to Catalyst expressions. This compiler will be injected automatically to spark extensions by setting `spark.plugins=com.nvidia.spark.SQLPlugin` and is disabled by default. |
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.
nit: I don't think we need this line. I don't think the user cares about the internal details of how we implemented it, just how to enable it, which is covered by the change below.
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.
Right, reasonable to me. Updated.
Signed-off-by: Allen Xu <allxu@nvidia.com>
build |
Sounds good, sorry I missed that. |
* Auto-register UDF extention when main plugin is set Signed-off-by: Allen Xu <allxu@nvidia.com> * Update udf-compiler descriptions in related docs Signed-off-by: Allen Xu <allxu@nvidia.com> * Remove unecessary lines to make more user-friendly Signed-off-by: Allen Xu <allxu@nvidia.com> Co-authored-by: Allen Xu <allxu@nvidia.com>
* Auto-register UDF extention when main plugin is set Signed-off-by: Allen Xu <allxu@nvidia.com> * Update udf-compiler descriptions in related docs Signed-off-by: Allen Xu <allxu@nvidia.com> * Remove unecessary lines to make more user-friendly Signed-off-by: Allen Xu <allxu@nvidia.com> Co-authored-by: Allen Xu <allxu@nvidia.com>
* Auto-register UDF extention when main plugin is set Signed-off-by: Allen Xu <allxu@nvidia.com> * Update udf-compiler descriptions in related docs Signed-off-by: Allen Xu <allxu@nvidia.com> * Remove unecessary lines to make more user-friendly Signed-off-by: Allen Xu <allxu@nvidia.com> Co-authored-by: Allen Xu <allxu@nvidia.com>
…IDIA#1102) Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Signed-off-by: Allen Xu allxu@nvidia.com
To deal with #688.
With this change, the udf-compiler extension will be registered automatically which has the same behavior as
SQLExecPlugin
.