chore(query engine): use serializable protobuf objects for physical plans #19638
+18,023
−3,437
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add graph functionality to Plan
Signed-off-by: Sophie Waldman <[email protected]>
sophiewaldman
changed the title
rfratto
added a commit
that referenced
this pull request
Nov 1, 2025
This adds two new packages, expressionpb and physicalpb, which are serializable representations of physical.Expression and physical.Plan, respectively. These packages include utility functions to convert between the protobuf representations and the planner types. A translation layer is used due to the complexity of integrating protobuf throughout the engine, as well as difficulties with finding a clean pattern to construct node types. #19638 took an initial attempt at fully integrating the protobuf types, but revealed that it is very challenging. While investiating the code, I observed that it's very clunky to work with the protobuf types, especailly with how often we rely on interface values. It's clear to me that we will want to eventually remove our translation layer, but doing it too soon means needing to update the entire engine code path twice. It is a much safer bet to start with a translation layer, find the right abstraction for constructing the protobuf, and then migrate once we have confidence in the pattern.
rfratto
added a commit
that referenced
this pull request
Nov 1, 2025
This adds two new packages, expressionpb and physicalpb, which are serializable representations of physical.Expression and physical.Plan, respectively. These packages include utility functions to convert between the protobuf representations and the planner types. A translation layer is used due to the complexity of integrating protobuf throughout the engine, as well as difficulties with finding a clean pattern to construct node types. #19638 took an initial attempt at fully integrating the protobuf types, but revealed that it is very challenging. While investiating the code, I observed that it's very clunky to work with the protobuf types, especailly with how often we rely on interface values. It's clear to me that we will want to eventually remove our translation layer, but doing it too soon means needing to update the entire engine code path twice. It is a much safer bet to start with a translation layer, find the right abstraction for constructing the protobuf, and then migrate once we have confidence in the pattern. Co-authored-by: Sophie Waldman <[email protected]>
rfratto
added a commit
that referenced
this pull request
Nov 1, 2025
This adds two new packages, expressionpb and physicalpb, which are serializable representations of physical.Expression and physical.Plan, respectively. These packages include utility functions to convert between the protobuf representations and the planner types. A translation layer is used due to the complexity of integrating protobuf throughout the engine, as well as difficulties with finding a clean pattern to construct node types. #19638 took an initial attempt at fully integrating the protobuf types, but revealed that it is very challenging. While investiating the code, I observed that it's very clunky to work with the protobuf types, especailly with how often we rely on interface values. It's clear to me that we will want to eventually remove our translation layer, but doing it too soon means needing to update the entire engine code path twice. It is a much safer bet to start with a translation layer, find the right abstraction for constructing the protobuf, and then migrate once we have confidence in the pattern. Co-authored-by: Sophie Waldman <[email protected]>
rfratto
added a commit
that referenced
this pull request
Nov 1, 2025
This adds two new packages, expressionpb and physicalpb, which are serializable representations of physical.Expression and physical.Plan, respectively. These packages include utility functions to convert between the protobuf representations and the planner types. A translation layer is used due to the complexity of integrating protobuf throughout the engine, as well as difficulties with finding a clean pattern to construct node types. #19638 took an initial attempt at fully integrating the protobuf types, but revealed that it is very challenging. While investiating the code, I observed that it's very clunky to work with the protobuf types, especailly with how often we rely on interface values. It's clear to me that we will want to eventually remove our translation layer, but doing it too soon means needing to update the entire engine code path twice. It is a much safer bet to start with a translation layer, find the right abstraction for constructing the protobuf, and then migrate once we have confidence in the pattern. Co-authored-by: Sophie Waldman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
We need to be able to serialize physical plans in order for the scheduler to be able to deliver plans to workers over the network. This PR replaces the existing Node and Expression objects with equivalent protobufs, and updates the code accordingly.
Which issue(s) this PR fixes:
Fixes https://github.com/grafana/loki-private/issues/2002
Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR