Replace panic in datafusion-expr crate
#3341
Conversation
|
@andygrove Please review. Note that the |
|
The Mac test probably needs to be rerun. |
Codecov Report
@@ Coverage Diff @@
## master #3341 +/- ##
==========================================
- Coverage 85.59% 85.58% -0.01%
==========================================
Files 296 296
Lines 54214 54220 +6
==========================================
+ Hits 46403 46404 +1
- Misses 7811 7816 +5
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
I restarted the mac test |
| if self.plan.schema().field_from_column(l).is_ok() | ||
| && right.schema().field_from_column(r).is_ok() | ||
| && can_hash(self.plan.schema().field_from_column(l).unwrap().data_type()) | ||
| if right.schema().field_from_column(r).is_ok() |
There was a problem hiding this comment.
This looks like it has changed the semantics of the code. Previously if field_from_column(l) was an Err then a different code path would have been followed and with this change, there would be an error instead.
There was a problem hiding this comment.
Ah I see. Really thanks! Fixed!
|
Benchmark runs are scheduled for baseline = 30fce22 and contender = e6378f4. e6378f4 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
6 participants
Which issue does this PR close?
Closes #3312.
Rationale for this change
Replace
panics with mostlyResults.What changes are included in this PR?
Are there any user-facing changes?
No