-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Migrate datafusion/sql
tests to insta, part2
#15499
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
Migrate datafusion/sql
tests to insta, part2
#15499
Conversation
This PR does two things:
|
datafusion/sql
tests to insta, part2
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.
This looks good to me -- thank you @qstommyshu -- very nice
); | ||
} | ||
|
||
#[test] | ||
fn cast_to_invalid_decimal_type_precision_0() { | ||
// precision == 0 | ||
{ |
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.
+1 for removing the uncessary level of nesting
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 was confused when looking at the code initially lol. I told myself, "Maybe this bracket has some use I don't know about" 🥲
FYI @blaginin |
Thanks again @qstommyshu |
* WIP: non-trivial cases migration * WIP: Fix projection expression in SQL integration test snapshot * clean up * rename `generate_plan` to `generate_logical_plan`
Which issue does this PR close?
datafusion/sql
tests toinsta
#15397, Migrate datafusion/sql tests to insta, part1 #15497 this is a part of Migrate datafusion/sql tests to insta,sql integrations #15484 breaking down.Rationale for this change
What changes are included in this PR?
This is the part 2 of #15484 breakdown, as the code changes in #15484 is too large. Part1: #15497
Are these changes tested?
Yes, I manually tested the before/after changes.
Are there any user-facing changes?
No