Refactor QueryStageExec in preparation for implementing map-side shuffle #459
Conversation
|
@edrevo fyi |
Codecov Report
@@ Coverage Diff @@
## master #459 +/- ##
==========================================
+ Coverage 75.30% 75.60% +0.30%
==========================================
Files 152 152
Lines 25275 25336 +61
==========================================
+ Hits 19033 19156 +123
+ Misses 6242 6180 -62
Continue to review full report at Codecov.
|
| } | ||
|
|
||
| impl QueryStageExec { | ||
| /// Create a new query stage | ||
| pub fn try_new( | ||
| job_id: String, | ||
| job_id: &str, |
There was a problem hiding this comment.
FWIW I think for "constructor methods" it is OK to take an owned String as the &str will be copied anyway.
There was a problem hiding this comment.
FWIW another pattern that I like is job_id: impl Into<String> so that the caller can pass in an owned String if they have one or a &str if they don't (or anything else that knows how to turn itself into a String)
| //TODO re-use code from RepartitionExec to split each batch into | ||
| // partitions and write to one IPC file per partition | ||
| // See https://github.com/apache/arrow-datafusion/issues/456 | ||
| unimplemented!() |
There was a problem hiding this comment.
Could use DataFusionError:: NotImplemented
Which issue does this PR close?
Closes #458
Rationale for this change
What changes are included in this PR?
QueryStageExec(which we can potentially move to DataFusion later on)Are there any user-facing changes?
No