-
Notifications
You must be signed in to change notification settings - Fork 241
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
Fix the failing test test_window
on Databricks
#2484
Conversation
By making the 'outReference' be lazy. Because on Databricks, it will get some invalid expressions in the 'projectList' if binding the reference on driver side. E.g. 'none#0L', 'none@1L'. Signed-off-by: Firestarman <[email protected]>
Verification on DB is running ... https://blossom.nvidia.com/sw-gpu-spark-jenkins/view/Testing/job/lc-db/ . |
build |
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.
still not sure about the root cause of abnormal expressions, we can get back to this if we see something-related again~
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.
I am happy that this was fixed, but it would have been nice to add a comment to the outReferences
explaining why it needs to be lazy. Often when there a subtle bugs like this it is easy for someone to accidentally undo the fix without a warning that there is a problem to be aware of.
Agree with Bobby, I tried to make sure in the new shim that we document changes made specific to Databricks, so if we can followup and add comment that would be great |
Made the PR #2496 to add the comments. |
By making the 'outReference' be lazy. Because on Databricks, it will get some invalid expressions in the 'projectList' if binding the reference on driver side. E.g. 'none#0L', 'none@1L'. Signed-off-by: Firestarman <[email protected]>
By making the 'outReference' be lazy. Because on Databricks, it will get some invalid expressions in the 'projectList' if binding the reference on driver side. E.g. 'none#0L', 'none@1L'. Signed-off-by: Firestarman <[email protected]>
This small PR is to fix the failing test
test_window
on Databricks by making theoutReference
be lazy.Because on Databricks, it will get some invalid expressions (e.g.
none#0L
,none@1L
.) in theprojectList
if binding the references on the driver side.closes #2372
Signed-off-by: Firestarman [email protected]