[SPARK-43302][SQL] Make Python UDAF an AggregateFunction #40739
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cloud-fan
force-pushed
the
python_udaf
branch
5 times, most recently
from
ba72bae
to
404aa7e
Compare
cloud-fan
force-pushed
the
python_udaf
branch
4 times, most recently
from
39b2dca
to
97378e7
Compare
cloud-fan
force-pushed
the
python_udaf
branch
2 times, most recently
from
dcb9775
to
84c8c37
Compare
cloud-fan
changed the title
cloud-fan
commented
Apr 27, 2023
| @@ -93,12 +93,19 @@ struct<> | |||
| -- !query output | |||
| org.apache.spark.sql.AnalysisException | |||
| { | |||
| "errorClass" : "MISSING_AGGREGATION", | |||
| "sqlState" : "42803", | |||
| "errorClass" : "GROUP_BY_POS_AGGREGATE", | |||
There was a problem hiding this comment.
now the error is the same as normal expressions.
cloud-fan
commented
Apr 27, 2023
| @@ -315,24 +315,9 @@ struct<1:int> | |||
| -- !query | |||
| SELECT 1 FROM range(10) HAVING udaf(id) > 0 | |||
There was a problem hiding this comment.
now we can run this query, which is the same as normal expressions.
HyukjinKwon
approved these changes
Apr 27, 2023
|
thanks for review, merging to master! |
LuciferYang
pushed a commit
to LuciferYang/spark
that referenced
this pull request
May 10, 2023
### What changes were proposed in this pull request? Today, `PythonUDF` can be used as an aggregate function according to the eval type. However, this is done in a tricky way, as `PythonUDF` does not extend `AggregateFunction` and we need to add special handling of it here and there. This is pretty error-prone, and we have hit issues such as apache#39824 This PR adds a new `PythonUDAF` expression which extends `AggregateFunction`. Now python udaf will be handled the same as normal aggregate functions, except for the places that we need to extract python functions. After this, we can remove most of the special handling of `PythonUDF`. ### Why are the changes needed? code cleanup ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? existing tests Closes apache#40739 from cloud-fan/python_udaf. Lead-authored-by: Wenchen Fan <wenchen@databricks.com> Co-authored-by: Hyukjin Kwon <gurwls223@apache.org> Co-authored-by: Wenchen Fan <cloud0fan@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
yaooqinn
pushed a commit
that referenced
this pull request
May 17, 2023
### What changes were proposed in this pull request? This is a followup of #40739 to do some code cleanup 1. remove the pattern `PYTHON_UDAF` as it's not used by any rule. 2. add `PythonFuncExpression.evalType` for convenience: catalyst rules (including third-party extensions) may want to get the eval type of a python function, no matter it's UDF or UDAF. 3. update the python profile to use `PythonUDAF.resultId` instead of `AggregateExpression.resultId`, to be consistent with `PythonUDF` ### Why are the changes needed? code cleanup ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? existing tests Closes #41142 from cloud-fan/follow. Lead-authored-by: Wenchen Fan <wenchen@databricks.com> Co-authored-by: Wenchen Fan <cloud0fan@gmail.com> Signed-off-by: Kent Yao <yao@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Today,
PythonUDFcan be used as an aggregate function according to the eval type. However, this is done in a tricky way, asPythonUDFdoes not extendAggregateFunctionand we need to add special handling of it here and there. This is pretty error-prone, and we have hit issues such as #39824This PR adds a new
PythonUDAFexpression which extendsAggregateFunction. Now python udaf will be handled the same as normal aggregate functions, except for the places that we need to extract python functions. After this, we can remove most of the special handling ofPythonUDF.Why are the changes needed?
code cleanup
Does this PR introduce any user-facing change?
no
How was this patch tested?
existing tests