[GitHub] [arrow-datafusion] Dandandan commented on a change in pull request #459: Refactor QueryStageExec in preparation for implementing map-side shuffle

[GitBox <gi...@apache.org>]

Dandandan commented on a change in pull request #459:
URL: https://github.com/apache/arrow-datafusion/pull/459#discussion_r642657693



##########
File path: ballista/rust/core/src/execution_plans/query_stage.rs
##########
@@ -31,26 +45,44 @@ use uuid::Uuid;
 #[derive(Debug, Clone)]
 pub struct QueryStageExec {
     /// Unique ID for the job (query) that this stage is a part of
-    pub job_id: String,
+    job_id: String,
     /// Unique query stage ID within the job
-    pub stage_id: usize,
+    stage_id: usize,
     /// Physical execution plan for this query stage
-    pub child: Arc<dyn ExecutionPlan>,
+    plan: Arc<dyn ExecutionPlan>,
+    /// Path to write output streams to
+    work_dir: String,
+    /// Optional shuffle output partitioning
+    shuffle_output_partitioning: Option<Partitioning>,
 }
 
 impl QueryStageExec {
     /// Create a new query stage
     pub fn try_new(
-        job_id: String,
+        job_id: &str,

Review comment:
       FWIW I think for "constructor methods" it is OK to take an owned `String` as the `&str` will be copied anyway.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org