You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "sandugood (via GitHub)" <gi...@apache.org> on 2026/05/03 14:30:20 UTC

[PR] feat: job plans optional rendering [datafusion-ballista]

sandugood opened a new pull request, #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650

   # Which issue does this PR close?
   
   Closes #1627.
   
    # Rationale for this change
   Add additional optional query parameters to the Scheduler's REST API to get data about the jobs. 
   
   # What changes are included in this PR?
   1. Added JobQueryParams struct for optional query parameters construction with two fields: include_logical and tree_rendered
   2. Based on optional parameters a JSON response is formed:
   - `include_logical` is true by default, but can be configured to be excluded from the response. Extracted by the name it is responsible for returning `logical_plan` to the user
   - `tree_rendered` is the query parameter that controls how `physical_plan` is rendered. If `tree_rendered` is set to `false` then it is rendered as `DisplayableExecutionPlan::new(job.physical_plan().as_ref()).indent(false)`, otherwise `displayable(job.physical_plan().as_ref()).tree_render().to_string()`, which can be easily rendered on the user side
   ```
   curl -s 'localhost:50050/api/job/JcYQYNY?tree_rendered=true' | jq -r ".physical_plan"
   ┌───────────────────────────┐
   │       AggregateExec       │
   │    --------------------   │
   │           aggr:           │
   │  min(test.c12), max(test  │
   │           .c12)           │
   │                           │
   │        group_by: c1       │
   │                           │
   │           mode:           │
   │      FinalPartitioned     │
   └─────────────┬─────────────┘
   ┌─────────────┴─────────────┐
   │      RepartitionExec      │
   │    --------------------   │
   │ partition_count(in->out): │
   │           1 -> 4          │
   │                           │
   │    partitioning_scheme:   │
   │      Hash([c1@0], 4)      │
   └─────────────┬─────────────┘
   ┌─────────────┴─────────────┐
   │       AggregateExec       │
   │    --------------------   │
   │           aggr:           │
   │  min(test.c12), max(test  │
   │           .c12)           │
   │                           │
   │        group_by: c1       │
   │       mode: Partial       │
   └─────────────┬─────────────┘
   ┌─────────────┴─────────────┐
   │         FilterExec        │
   │    --------------------   │
   │         predicate:        │
   │  c11 > 0.1 AND c11 < 0.9  │
   └─────────────┬─────────────┘
   ┌─────────────┴─────────────┐
   │       DataSourceExec      │
   │    --------------------   │
   │          files: 1         │
   │        format: csv        │
   └───────────────────────────┘
   
   ```
   
   # Are there any user-facing changes?
   Additional query parameters added to the Scheduler's REST API. 


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#issuecomment-4366406293

   @milenkovicm would be glad if you could review this change
   LogicalPlan can't be used directly in the displayable() or so it is in the current DF implementation


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on code in PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#discussion_r3184641012


##########
ballista/scheduler/src/api/handlers.rs:
##########
@@ -203,6 +205,12 @@ pub struct QueryStageSummary {
     pub tasks: Vec<Option<TaskSummary>>,
 }
 
+#[derive(Debug, serde::Deserialize, Default)]
+pub struct JobQueryParams {
+    pub include_logical: Option<bool>,

Review Comment:
   Considering the fact, that LogicalPlan can't be rendered using the `displayable() ` func, users might only be interested in the tree-rendered style of the physical plan. However, getting logical plan info is toggled by default, so users do not face any breaking changes in the API



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on code in PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#discussion_r3184650783


##########
ballista/scheduler/src/api/handlers.rs:
##########
@@ -203,6 +205,12 @@ pub struct QueryStageSummary {
     pub tasks: Vec<Option<TaskSummary>>,
 }
 
+#[derive(Debug, serde::Deserialize, Default)]
+pub struct JobQueryParams {
+    pub include_logical: Option<bool>,

Review Comment:
   But we can also keep the old API, where logical_plan is just sent to the end user by default (no changes). Wdyt @milenkovicm?



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on code in PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#discussion_r3178481196


##########
ballista/scheduler/src/state/mod.rs:
##########
@@ -444,17 +444,15 @@ impl<T: 'static + AsLogicalPlan, U: 'static + AsExecutionPlan> SchedulerState<T,
         } else {
             None
         };
-
+        // let logical_plan_tree = DisplayableExecutionPlan::new(plan.as_ref());

Review Comment:
   nit, can we remove it 



##########
ballista/scheduler/src/test_utils.rs:
##########
@@ -973,7 +974,7 @@ pub async fn test_two_aggregations_plan(partition: usize) -> StaticExecutionGrap
         Arc::new(SessionConfig::new_with_ballista()),
         &mut planner,
         None,
-        None,
+        cloned_plan,

Review Comment:
   i guess we do not need this paramter 



##########
ballista/scheduler/src/state/aqe/mod.rs:
##########
@@ -130,7 +130,7 @@ impl AdaptiveExecutionGraph {
         queued_at: u64,
         session_config: Arc<SessionConfig>,
         logical_plan: Option<String>,
-        physical_plan: Option<String>,
+        physical_plan: Arc<dyn ExecutionPlan>,

Review Comment:
   is `phisical_plan` duplication of `plan`?



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: rest api supports plan tree rendering [datafusion-ballista]

Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#issuecomment-4378116804

   @martin-g any suggestions ? 


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on code in PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#discussion_r3184877415


##########
ballista/scheduler/src/api/handlers.rs:
##########
@@ -203,6 +205,12 @@ pub struct QueryStageSummary {
     pub tasks: Vec<Option<TaskSummary>>,
 }
 
+#[derive(Debug, serde::Deserialize, Default)]
+pub struct JobQueryParams {
+    pub include_logical: Option<bool>,

Review Comment:
   Yeah, I think that it makes perfect sense for now. Thanks @milenkovicm 



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on code in PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#discussion_r3184641012


##########
ballista/scheduler/src/api/handlers.rs:
##########
@@ -203,6 +205,12 @@ pub struct QueryStageSummary {
     pub tasks: Vec<Option<TaskSummary>>,
 }
 
+#[derive(Debug, serde::Deserialize, Default)]
+pub struct JobQueryParams {
+    pub include_logical: Option<bool>,

Review Comment:
   Considering the fact, that LogicalPlan can't be rendered using the `displayable()` func, users might only be interested in the tree-rendered style of the physical plan. However, getting logical plan info is toggled by default, so users do not face any breaking changes in the API



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on code in PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#discussion_r3184584194


##########
ballista/scheduler/src/api/handlers.rs:
##########
@@ -203,6 +205,12 @@ pub struct QueryStageSummary {
     pub tasks: Vec<Option<TaskSummary>>,
 }
 
+#[derive(Debug, serde::Deserialize, Default)]
+pub struct JobQueryParams {
+    pub include_logical: Option<bool>,

Review Comment:
   In which case we would not include logical plan? What's the idea behind this flag?



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: rest api supports plan tree rendering [datafusion-ballista]

Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#issuecomment-4378112801

   I thought that `render_tree` seemed clearer (I'm always unsure about the namings) as parameter name, so I switched to it.
   
   Added this query param to the `/api/job/{job_id}/stages` endpoint. Looks like this:
   
   ```
   ~ curl -s localhost:50050/api/job/Za9mMBI/stages?render_tree=true | jq -r ".stages.[0].stage_plan"
   ┌───────────────────────────┐
   │   SortShuffleWriterExec   │
   │    --------------------   │
   │       partitioning:       │
   │      Hash([c1@0], 4)      │
   └─────────────┬─────────────┘
   ┌─────────────┴─────────────┐
   │       AggregateExec       │
   │    --------------------   │
   │           aggr:           │
   │  min(test.c12), max(test  │
   │           .c12)           │
   │                           │
   │        group_by: c1       │
   │       mode: Partial       │
   └─────────────┬─────────────┘
   ┌─────────────┴─────────────┐
   │         FilterExec        │
   │    --------------------   │
   │         predicate:        │
   │  c11 > 0.1 AND c11 < 0.9  │
   └─────────────┬─────────────┘
   ┌─────────────┴─────────────┐
   │       DataSourceExec      │
   │    --------------------   │
   │          files: 1         │
   │        format: csv        │
   └───────────────────────────┘
   
   
   ```


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on code in PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#discussion_r3184844374


##########
ballista/scheduler/src/api/handlers.rs:
##########
@@ -203,6 +205,12 @@ pub struct QueryStageSummary {
     pub tasks: Vec<Option<TaskSummary>>,
 }
 
+#[derive(Debug, serde::Deserialize, Default)]
+pub struct JobQueryParams {
+    pub include_logical: Option<bool>,

Review Comment:
   I'm struggling to find a case where we would disable logical plan. Let's not give that option for now and revert to previous behaviour if you agree @sandugood 



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: rest api supports plan tree rendering [datafusion-ballista]

Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#issuecomment-4450453505

   While adding support for tree rendering to the TUI app I noticed that the width of the ASCII chart is rather narrow.
   I didn't find an easy way to make it wider. 
   
   <img width="1532" height="791" alt="image" src="https://github.com/user-attachments/assets/ecc6776f-6c5c-4368-8528-6cc7502a6cc4" />
   


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: rest api supports plan tree rendering [datafusion-ballista]

Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm merged PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: job plans optional rendering [datafusion-ballista]

Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on code in PR #1650:
URL: https://github.com/apache/datafusion-ballista/pull/1650#discussion_r3178645388


##########
ballista/scheduler/src/state/aqe/mod.rs:
##########
@@ -130,7 +130,7 @@ impl AdaptiveExecutionGraph {
         queued_at: u64,
         session_config: Arc<SessionConfig>,
         logical_plan: Option<String>,
-        physical_plan: Option<String>,
+        physical_plan: Arc<dyn ExecutionPlan>,

Review Comment:
   Actually, yeah. It was the planned before the `transform_down` in the `SchedulerState`'s `submit_job` function
   Now using the physical plan we get after transformations. 



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org