Skip to content
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

Merged
merged 3 commits into from
Nov 16, 2020

Conversation

wjxiz1992
Copy link
Collaborator

@wjxiz1992 wjxiz1992 commented Nov 12, 2020

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.

Signed-off-by: Allen Xu <allxu@nvidia.com>
@wjxiz1992 wjxiz1992 self-assigned this Nov 12, 2020
@wjxiz1992 wjxiz1992 changed the title Auto-register UDF extention when main plugin is set [WIP] Auto-register UDF extention when main plugin is set Nov 12, 2020
@wjxiz1992 wjxiz1992 changed the title [WIP] Auto-register UDF extention when main plugin is set Auto-register UDF extention when main plugin is set Nov 12, 2020
revans2
revans2 previously approved these changes Nov 12, 2020
Copy link
Collaborator

@revans2 revans2 left a 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

@revans2
Copy link
Collaborator

revans2 commented Nov 12, 2020

build

Copy link
Collaborator

@abellina abellina left a 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")

@wjxiz1992
Copy link
Collaborator Author

wjxiz1992 commented Nov 13, 2020

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.
Can we keep the UDF test suites independent and leave the extension there?

@revans2
Copy link
Collaborator

revans2 commented Nov 13, 2020

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>
revans2
revans2 previously approved these changes Nov 13, 2020
@@ -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.
Copy link
Collaborator

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.

Copy link
Collaborator Author

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>
@revans2
Copy link
Collaborator

revans2 commented Nov 13, 2020

build

@abellina
Copy link
Collaborator

Removing current extension requires adding main sql plugin config. But that will fail the OpcodeSuite.
Can we keep the UDF test suites independent and leave the extension there?

Sounds good, sorry I missed that.

@sameerz sameerz added the feature request New feature or request label Nov 15, 2020
@wjxiz1992 wjxiz1992 merged commit 9822bbf into NVIDIA:branch-0.3 Nov 16, 2020
sperlingxx pushed a commit to sperlingxx/spark-rapids that referenced this pull request Nov 20, 2020
* 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>
@wjxiz1992 wjxiz1992 deleted the auto-register-udf branch December 18, 2020 03:49
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
* 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>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
* 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>
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
…IDIA#1102)

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants