-
Notifications
You must be signed in to change notification settings - Fork 240
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
Support aggregation on NullType in RunningWindowExec #2722
Support aggregation on NullType in RunningWindowExec #2722
Conversation
Signed-off-by: Mithun RK <mythrocks@gmail.com>
build |
Do we know if we need NullType support for 21.06? |
#2596 didn't ship in 21.06, so I think it should be alright for this to be left out of 21.06 as well. |
@@ -105,7 +105,7 @@ class Spark301dbShims extends Spark301Shims { | |||
"Databricks-specific window function exec, for \"running\" windows, " + | |||
"i.e. (UNBOUNDED PRECEDING TO CURRENT ROW)", | |||
ExecChecks( | |||
TypeSig.commonCudfTypes + TypeSig.DECIMAL + | |||
TypeSig.commonCudfTypes + TypeSig.DECIMAL + TypeSig.Null + |
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.
found a typo here. I will draft a fix
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.
Oh, yikes!
I noticed that when testing on Databricks 7.3. I honestly thought I'd fixed that already.
I can post a fix shortly.
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.
#2729 just post one, can you help take a look? thx~
Fixes #2715.
#2596 extended
WindowExec
's meta to allow for aggregations onNullType
input. E.g.test_window_running()
andtest_window_running_no_part()
integration tests were added to exercise this path.#2596 did not include a change to enable
NullType
aggregations in the Databricks Shims, causing thetest_window_running*()
tests to fail.This commit should resolve the failure.
Signed-off-by: Mithun RK mythrocks@gmail.com