-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-28280][PYTHON][SQL][TESTS][FOLLOW-UP] Add UDF cases into group by clause in 'udf-group-by.sql' #25360
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
|
@HyukjinKwon fyi. |
HyukjinKwon
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.
Thanks for updating it.
| -- The following query will make Scala UDF work, but Python and Pandas udfs will fail with an AnalysisException. | ||
| -- The query should be added after SPARK-28445. | ||
| -- SELECT udf(a + 1), udf(COUNT(b)) FROM testData GROUP BY udf(a + 1); | ||
| SELECT udf(a + 1), udf(COUNT(b)) FROM testData GROUP BY udf(a + 1); |
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.
Actually, @skonto, do you mind if I ask to put udf in some more places in this file? I just realised that this test cases for group-by basically and might be good to add udfs more at group-by places. Looks good otherwise.
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.
Ok I can do that, need some time, will get it fixed by tomorrow.
|
Test build #108661 has finished for PR 25360 at commit
|
6f05649 to
245a4d9
Compare
|
@HyukjinKwon I updated the PR, let me know if I need to add anything else. |
HyukjinKwon
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.
LGTM if the tests pass
|
Test build #108708 has finished for PR 25360 at commit
|
sql/core/src/test/resources/sql-tests/inputs/udf/udf-group-by.sql
Outdated
Show resolved
Hide resolved
245a4d9 to
02cc4a2
Compare
|
@HyukjinKwon done. |
|
Test build #108973 has finished for PR 25360 at commit
|
What changes were proposed in this pull request?
This PR is a followup of a fix as described in here: #25215 (comment)
Diff comparing to 'group-by.sql'
How was this patch tested?
Tested as instructed in SPARK-27921.