Uh oh!
There was an error while loading. Please reload this page.
Minor: add ExecutionPlan.execute() snippet for sync methods - #8010
Minor: add ExecutionPlan.execute() snippet for sync methods#8010comphead wants to merge 3 commits into
ExecutionPlan.execute() snippet for sync methods#8010Conversation
| /// | ||
| /// fn print_plan_exec() -> Result<(), DataFusionError> { | ||
| /// let stream = node.execute(0, task_ctx)?; | ||
| /// futures::stream::once(async move { |
There was a problem hiding this comment.
This stream gets immediately dropped and therefore will never run?
There was a problem hiding this comment.
okay, I'll include another use case with amending stream and returning it back to the user. Like in https://github.com/apache/arrow-datafusion/blob/main/datafusion/physical-plan/src/sorts/sort.rs#L895
@tustvold I believe you mean exactly that?
There was a problem hiding this comment.
I mean as written this example doesn't do anything and is therefore just misleading.
Like in https://github.com/apache/arrow-datafusion/blob/main/datafusion/physical-plan/src/sorts/sort.rs#L895
Yes, that is I think a good example of the pattern
tustvold
commented
Oct 31, 2023
I tried to encode what I was expecting in #8013 PTAL |
comphead
commented
Oct 31, 2023
Closed in favor of #8013 |
Which issue does this PR close?
Closes #.
Rationale for this change
Very often the question arises on debugging or using record stream in sync context. Adding a code snippet for
ExecutionPlan.execute()What changes are included in this PR?
A code snippet
Are these changes tested?
Are there any user-facing changes?