Open
Conversation
Introduces `PlaceholderExpr`, allowing placeholder parameters to be preserved in the physical plan. Previously, placeholders had to be resolved to literals before physical planning.
Author
|
@askalt, you've already read the code. Could you take another look at it? |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Rationale for this change
Part of #14342.
Previously, DataFusion required all placeholders to be resolved to literal values before physical planning. This limitation made it difficult to work in systems where physical plans might be cached or reused, or where resolution happens later in the execution pipeline.
By introducing
PlaceholderExprat the physical level, we allow the physical planner to handle unresolved placeholders. This is a logical next step for PR #20009 and part of the effort to improve plan reuse support in DataFusion.What changes are included in this PR?
PlaceholderExpras a new physical expression.PlaceholderExprwhen encounteringExpr::Placeholder.Are these changes tested?
Yes, SLTs have been added, which verifies that placeholders are correctly preserved in physical plans across a wide range of SQL queries.
Are there any user-facing changes?
Yes, users can now generate physical plans for queries containing unresolved placeholders. Attempting to execute these plans without resolving the placeholders will still result in an error, but the planning phase no longer requires their resolution.