Skip to content

refactor: consolidate multi-step planning, assembly and execution into QueryPlanner type - #1482

Draft
jkaczman wants to merge 5 commits into
mainfrom
refactor-multistep
Draft

refactor: consolidate multi-step planning, assembly and execution into QueryPlanner type#1482
jkaczman wants to merge 5 commits into
mainfrom
refactor-multistep

Conversation

@jkaczman

@jkaczman jkaczman commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@sgrif and I have been working on #1096 (subqueries), and as the first step to getting there, discussed how we can create an architecture for multi-step that...

  • (1) reduces duplication,
  • (2) concentrates all relevant code into one area,
  • (3) clearly articulates query dependencies (as in, we cannot construct a query without receiving the responses of a previous query). An example of being we cannot construct the INSERT for Sharding Key Updates without the DELETE ... RETURNING * DataRow returning. Another example being, of course, subqueries.
  • (4) doesn't do more than necessary; as in, we only need the Parse/Bind handled dynamic Steps, we don't need to pre-make requests for each before execution-time,
  • (5) removing unnecessary code, e.g. the InPlace variant, so that we can just receive the QueryPlanner and handle it, and also, making OffsetPlan independent of this new struct, as it's not relevant (and can just be passed straight through)
  • (6) a nice abstraction for (1) knowing when to forward messages to the Client, and (2) determining how to aggregate them (especially in dynamic situations)
  • (7) running checks during planning instead of having a check in the parser and deferring more checks until later, which we can do because we have the QueryEngineContext to work with now.
  • ... and more

No tests were removed. Added a couple of tests for a few things I wanted to make sure worked.

A lot of the diff is from moving pre-existing functions/tests around.

  • pgdog/src/frontend/router/parser/rewrite/statement/update.rs => pgdog/src/frontend/client/query_engine/multi_step/test/sharding_key_update.rs
  • pgdog/src/frontend/router/parser/rewrite/statement/insert.rs => pgdog/src/frontend/client/query_engine/multi_step/test/split_insert.rs

TODO: Cache insert splits / subqueries / etc (relevant benchmark: #1484)

@jkaczman
jkaczman requested a review from levkk September 2, 2026 16:02
Comment thread pgdog/src/frontend/client/query_engine/multi_step/types.rs Outdated

/// Splitting of multi-tuple INSERT statements into
/// multiple queries.
pub(crate) insert_split: Vec<InsertSplit>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We used to cache the rewrite previously. Now, we're performing it on each request, prepared statement or not. I suspect that would add some performance overhead to this step, worth benchmarking.

@jkaczman
jkaczman marked this pull request as ready for review September 2, 2026 20:27
@jkaczman
jkaczman marked this pull request as draft September 2, 2026 22:19
@sgrif sgrif self-assigned this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants