You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/08/28 13:04:38 UTC

[PR] Minor: Improve ExecutionMode documentation [datafusion]

alamb opened a new pull request, #12214:
URL: https://github.com/apache/datafusion/pull/12214

   N/A
   
   ## Rationale for this change
   
   While working with @ozankabak  and @berkaysynnada on https://github.com/apache/datafusion/pull/12174.  I found myself referring several times to the `ExecutionMode` documentation and had to re-read it a few times 
   
   ## What changes are included in this PR?
   Try to improve the docs
   
   <!--
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   ## Are these changes tested?
   
   <!--
   We typically require tests for all PRs in order to:
   1. Prevent the code from being accidentally broken by subsequent changes
   2. Serve as another way to document the expected behavior of the code
   
   If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
   -->
   
   ## Are there any user-facing changes?
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!--
   If there are any breaking changes to public APIs, please add the `api change` label.
   -->
   


-- 
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] Minor: Improve ExecutionMode documentation [datafusion]

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

   Thanks again @ozankabak 


-- 
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] Minor: Improve ExecutionMode documentation [datafusion]

Posted by "ozankabak (via GitHub)" <gi...@apache.org>.
ozankabak commented on code in PR #12214:
URL: https://github.com/apache/datafusion/pull/12214#discussion_r1734711304


##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -482,24 +482,41 @@ impl ExecutionPlanProperties for &dyn ExecutionPlan {
     }
 }
 
-/// Describes the execution mode of an operator's resulting stream with respect
-/// to its size and behavior. There are three possible execution modes: `Bounded`,
-/// `Unbounded` and `PipelineBreaking`.
-#[derive(Clone, Copy, PartialEq, Debug)]
+/// Describes the execution mode of the result of calling
+/// [`ExecutionPlan::execute`] with respect to its size and behavior.
+///
+/// The mode of the execution plan is determined by the mode of its input
+/// execution plans and the details of the operator itself. For example, a
+/// `FilterExec` operator will have the same execution mode as its input, but a
+/// `SortExec` operator may have a different execution mode than its input,
+/// depending on how the input stream is sorted.
+///
+/// There are three possible execution modes: `Bounded`, `Unbounded` and
+/// `PipelineBreaking`.
+# [derive(Clone, Copy, PartialEq, Debug)]
 pub enum ExecutionMode {
-    /// Represents the mode where generated stream is bounded, e.g. finite.
+    /// The stream is bounded / finite.
+    ///
+    /// In this case the stream will eventually return `None` to indicate that
+    /// there are no more records to process.
     Bounded,
-    /// Represents the mode where generated stream is unbounded, e.g. infinite.
-    /// Even though the operator generates an unbounded stream of results, it
-    /// works with bounded memory and execution can still continue successfully.
+    /// The stream is unbounded / infinite.
     ///
-    /// The stream that results from calling `execute` on an `ExecutionPlan` that is `Unbounded`
-    /// will never be done (return `None`), except in case of error.
+    /// In this case, the stream will never be done (never return `None`),
+    /// except in case of error.
+    ///
+    /// This mode is often used in "Steaming" use cases where data is
+    /// incrementally processed as it arrives.
+    ///
+    /// Note that even though the operator generates an unbounded stream of
+    /// results, it can execute with bounded memory and incrementally produces
+    /// output.
     Unbounded,
-    /// Represents the mode where some of the operator's input stream(s) are
-    /// unbounded; however, the operator cannot generate streaming results from
-    /// these streaming inputs. In this case, the execution mode will be pipeline
-    /// breaking, e.g. the operator requires unbounded memory to generate results.
+    /// Some of the operator's input stream(s) are unbounded, but the  operator
+    /// cannot generate streaming results from these streaming inputs.
+    ///
+    /// In this case, the execution mode will be pipeline breaking, e.g. the
+    /// operator requires unbounded memory to generate results.

Review Comment:
   ```suggestion
       /// Some of the operator's input stream(s) are unbounded, but the operator
       /// cannot generate streaming results from these streaming inputs.
       ///
       /// In this case, the execution mode will be pipeline breaking, e.g. the
       /// operator requires unbounded memory to generate results. This
       /// information is used by the planner when performing sanity checks
       /// on plans processings unbounded data sources.
   ```



-- 
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] Minor: Improve ExecutionMode documentation [datafusion]

Posted by "ozankabak (via GitHub)" <gi...@apache.org>.
ozankabak commented on code in PR #12214:
URL: https://github.com/apache/datafusion/pull/12214#discussion_r1734743353


##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -482,24 +482,41 @@ impl ExecutionPlanProperties for &dyn ExecutionPlan {
     }
 }
 
-/// Describes the execution mode of an operator's resulting stream with respect
-/// to its size and behavior. There are three possible execution modes: `Bounded`,
-/// `Unbounded` and `PipelineBreaking`.
-#[derive(Clone, Copy, PartialEq, Debug)]
+/// Describes the execution mode of the result of calling
+/// [`ExecutionPlan::execute`] with respect to its size and behavior.
+///
+/// The mode of the execution plan is determined by the mode of its input
+/// execution plans and the details of the operator itself. For example, a
+/// `FilterExec` operator will have the same execution mode as its input, but a
+/// `SortExec` operator may have a different execution mode than its input,
+/// depending on how the input stream is sorted.
+///
+/// There are three possible execution modes: `Bounded`, `Unbounded` and
+/// `PipelineBreaking`.
+# [derive(Clone, Copy, PartialEq, Debug)]

Review Comment:
   ```suggestion
   #[derive(Clone, Copy, PartialEq, Debug)]
   ```
   Whitespace creates a fmt error



-- 
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] Minor: Improve ExecutionMode documentation [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #12214:
URL: https://github.com/apache/datafusion/pull/12214#discussion_r1740094486


##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -482,24 +482,41 @@ impl ExecutionPlanProperties for &dyn ExecutionPlan {
     }
 }
 
-/// Describes the execution mode of an operator's resulting stream with respect
-/// to its size and behavior. There are three possible execution modes: `Bounded`,
-/// `Unbounded` and `PipelineBreaking`.
-#[derive(Clone, Copy, PartialEq, Debug)]
+/// Describes the execution mode of the result of calling
+/// [`ExecutionPlan::execute`] with respect to its size and behavior.
+///
+/// The mode of the execution plan is determined by the mode of its input
+/// execution plans and the details of the operator itself. For example, a
+/// `FilterExec` operator will have the same execution mode as its input, but a
+/// `SortExec` operator may have a different execution mode than its input,
+/// depending on how the input stream is sorted.
+///
+/// There are three possible execution modes: `Bounded`, `Unbounded` and
+/// `PipelineBreaking`.
+# [derive(Clone, Copy, PartialEq, Debug)]

Review Comment:
   Thank you -- sorry about that



-- 
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] Minor: Improve ExecutionMode documentation [datafusion]

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


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