Fix Native Plan Checker for CTAS and Insert queries#25115
Merged
aditi-pandit merged 4 commits intoprestodb:masterfrom Jun 6, 2025
Merged
Fix Native Plan Checker for CTAS and Insert queries#25115aditi-pandit merged 4 commits intoprestodb:masterfrom
aditi-pandit merged 4 commits intoprestodb:masterfrom
Conversation
tdcmeehan
force-pushed
the
validate_inserts
branch
5 times, most recently
from
5d3fc14 to
d573908
Compare
tdcmeehan
changed the title
tdcmeehan
requested review from
a team,
elharo,
feilong-liu,
jaystarshot and
vivek-bharathan
as code owners
prestodb-ci
requested review from
a team,
infvg and
nmahadevuni
and removed request for
a team and
ZacBlanco
BryanCutler
previously approved these changes
May 27, 2025
Contributor
BryanCutler
left a comment
BryanCutler
left a comment
There was a problem hiding this comment.
Looks better, thanks for the update to this!
| runValidation(removeTableWriter(planFragment)); | ||
| } | ||
|
|
||
| private SimplePlanFragment removeTableWriter(SimplePlanFragment planFragment) |
Contributor
There was a problem hiding this comment.
It might be good to add a note as to why the TableWriterNode gets replaced
tdcmeehan
force-pushed
the
validate_inserts
branch
from
d573908 to
0be61ba
Compare
|
|
||
| // Launch distributed runner. | ||
| DistributedQueryRunner queryRunner = (DistributedQueryRunner) PrestoNativeQueryRunnerUtils.createQueryRunner(false, false, false, false); | ||
| DistributedQueryRunner queryRunner = (DistributedQueryRunner) PrestoNativeQueryRunnerUtils.createQueryRunner(false, false, false, false, ImmutableMap.of()); |
Contributor
There was a problem hiding this comment.
#25120 for QueryRunner with builders should soon be submit. Please refactor your code according to that.
| runValidation(removeTableWriter(planFragment)); | ||
| } | ||
|
|
||
| private SimplePlanFragment removeTableWriter(SimplePlanFragment planFragment) |
6 tasks
tdcmeehan
force-pushed
the
validate_inserts
branch
2 times, most recently
from
7dac494 to
b8ea423
Compare
ZacBlanco
requested changes
Jun 5, 2025
tdcmeehan
force-pushed
the
validate_inserts
branch
from
b8ea423 to
4843ba1
Compare
ZacBlanco
approved these changes
Jun 5, 2025
Previously, these queries would always fail because the root of the plan was TableWriterMerge. Now, the plan is modified to swap out a dummy project node for the TableWriterNode. This is because TableWriterNode relies on information that is not known until the scheduling phase.
tdcmeehan
force-pushed
the
validate_inserts
branch
from
4843ba1 to
8594d73
Compare
6 tasks
This was referenced Jul 4, 2025
This was referenced Jul 24, 2025
6 tasks
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
5 participants
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.
Description
Currently writes are broken due to how the plan is presumed to be shaped by the C++ worker.
Motivation and Context
The sidecar should validate as many query shapes as possible for it to be effective.
Impact
Better validation by the sidecar.
Test Plan
Tests have been included.
Contributor checklist
Release Notes