Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support physical plan reusage #2

Merged
merged 6 commits into from
Feb 10, 2025

Conversation

askalt
Copy link

@askalt askalt commented Feb 4, 2025

To reuse physical plans 2 things were added:

  1. Support for physical placeholders. It is done by adding a new physical expression. Param values are added to the execution context. On the execute(...) phase each ExecutionPlan is responsible for resolving placeholders in its own expressions.

  2. Move metrics from ExecutionPlan to the execution context. To share physical plans across executions we need to
    place metrics in some other place. When plan is executed metrics set is registered in the context by the plan address. To display a plan with metrics one should provide the task context, where metrics associated with the plan are stored.

Also, fmt errors are fixed and applied several clippy suggestions.

@askalt askalt force-pushed the askalt/physical-placehdolders branch 3 times, most recently from d137f2a to 5b53af5 Compare February 4, 2025 20:01
@askalt askalt force-pushed the askalt/physical-placehdolders branch 2 times, most recently from fa55038 to da18987 Compare February 5, 2025 11:57
@askalt askalt force-pushed the askalt/physical-placehdolders branch from da18987 to 23beb3c Compare February 5, 2025 12:03
@askalt askalt changed the title Askalt/physical placehdolders Support physical reusage Feb 5, 2025
@askalt askalt changed the title Support physical reusage Support physical plan reusage Feb 5, 2025
@askalt askalt force-pushed the askalt/physical-placehdolders branch from 23beb3c to 883e0f9 Compare February 5, 2025 12:36
@0x501D 0x501D requested a review from WeCodingNow February 5, 2025 12:37
@askalt askalt force-pushed the askalt/physical-placehdolders branch from 883e0f9 to 8c23e1f Compare February 5, 2025 12:39
This patchs adds a support for placeholders on the physical expression
level. It allows to use generic plans and then resolve placeholders on the
execution stage.

Placeholders are resolved on the `execution(...)` phase. Each `ExecutionPlan`
is responsible for resolving placeholders in it's own expressions.

Example:
```sh
> create table a(x int);

> explain select x + $1 from a where x > $2;
+---------------+-------------------------------------------------------------------------+
| plan_type     | plan                                                                    |
+---------------+-------------------------------------------------------------------------+
| logical_plan  | Projection: a.x + $1                                                    |
|               |   Filter: a.x > $2                                                      |
|               |     TableScan: a projection=[x]                                         |
| physical_plan | ProjectionExec: expr=[x@0 + $1 as a.x + $1]                             |
|               |   RepartitionExec: partitioning=RoundRobinBatch(16), input_partitions=1 |
|               |     CoalesceBatchesExec: target_batch_size=8192                         |
|               |       FilterExec: x@0 > $2                                              |
|               |         MemoryExec: partitions=1, partition_sizes=[0]                   |
|               |                                                                         |
+---------------+-------------------------------------------------------------------------+
```
To share physical plans across executions we need to place
metrics in some other place. This patch moves them to the
task context. When plan is scanned metrics set is registered
in the context by the plan address.

To display a plan with metrics one should provide the task context,
where metrics associated with the plan are stored.

Also, fmt errors are fixed and applied several clippy suggestions.
@askalt askalt force-pushed the askalt/physical-placehdolders branch from 8c23e1f to 574ce4e Compare February 5, 2025 13:34
@askalt askalt force-pushed the askalt/physical-placehdolders branch 2 times, most recently from 893ca43 to 1b777b2 Compare February 5, 2025 14:21
@askalt askalt force-pushed the askalt/physical-placehdolders branch 2 times, most recently from 44ed854 to df20563 Compare February 5, 2025 14:55
We want to run on external PRs, but not on our own internal
PRs as they'll be run by the push to the branch.

The main trick is described here:
Dart-Code/Dart-Code#2375

Also we want to run it always for manually triggered workflows.
We do not support wasm datafusion for now, so let's
disable this job.
@askalt askalt force-pushed the askalt/physical-placehdolders branch from df20563 to 3300604 Compare February 5, 2025 16:02
@0x501D 0x501D requested a review from ole-baranov February 6, 2025 14:56
Copy link

@WeCodingNow WeCodingNow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@0x501D 0x501D merged commit 66220fb into release-42.0.0 Feb 10, 2025
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants