-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-24957][SQL][FOLLOW-UP] Clean the code for AVERAGE #21951
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
|
@gatorsmile, thanks! I am a bot who has found some folks who might be able to help with the review:@rxin, @cloud-fan and @ueshin |
|
Test build #93918 has finished for PR 21951 at commit
|
|
Why would we want to use the DSL here? Do we use it in other expressions? |
|
This will simplify the code and improve the readability. We can do the same in the other expression. |
|
LGTM.
…On Thu, Aug 2, 2018 at 1:14 AM Xiao Li ***@***.***> wrote:
This will simplify the code and improve the readability. We can do the
same in the other expression.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#21951 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AATvPIEkrQyLFUXLOl66_94V2f6MY7Obks5uMoqrgaJpZM4VrcXh>
.
|
|
LGTM as well. Thanks a lot! |
|
LGTM. shall we create a JIRA ticket to apply this to other |
|
Thanks! Merged to master. Please ignore the last commit. The last commit is not merged. |
|
Test build #93956 has finished for PR 21951 at commit
|
What changes were proposed in this pull request?
This PR is to refactor the code in AVERAGE by dsl.
How was this patch tested?
N/A