-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Insta for enforce_distrubution (easy ones) #18248
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
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.
Thank you @blaginin
I will admit I didn't review each line of this PR in super detail, but I reviewd all the lines and I spot checked a few of the tests in more detail and it looked really good.
Thank you for pusing along this project
| assert_eq!(captured_join_type, join_type.to_string()); | ||
|
|
||
| insta::allow_duplicates! {insta::assert_snapshot!(modified_plan, @r" | ||
| HashJoinExec: mode=Partitioned, join_type=..., on=[(AA@1, a1@5), (B@2, b1@6), (C@3, c@2)] |
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.
nice
|
thank you!! now the last one 🥁 |
So close! |
- part of apache#15791 All easy cases from apache#18185 (that are nicely-ish displayed in git diff). Note on preserving comments: if it was note about what should happen (or what will be tested), it's placed on top of the snapshot. If that's something that comments part of the plan, I put it below the plan
coretests toinsta#15791All easy cases from #18185 (that are nicely-ish displayed in git diff).
Note on preserving comments: if it was note about what should happen (or what will be tested), it's placed on top of the snapshot. If that's something that comments part of the plan, I put it below the plan