physicalplan: don't combine render nodes into one processor #97554
Conversation
|
This change is |
There was a problem hiding this comment.
Nice find! 
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball)
During planning for a render node, the physical planning logic checks whether the last created processor rendered its own expressions, adds its render expressions to the processor if not. The check is necessary to handle the case when expressions in a parent render reference those from a child render. However, render nodes also handle projecting input columns: both removing them and reordering them. Correct planning of the processors depends on the columns at each step being in a particular order. Since physical planning didn't check whether the existing processor projected its columns in some way, the parent render could be combined with its child. This could lead to panics in the parent render expressions when the columns weren't projected as expected. This patch fixes the issue by adding the projection check before allowing the render planning code to add to the previous processor. Fixes cockroachdb#95633 Release note (bug fix): Fixed a rare bug existing since before 22.1 that could cause a projected expression to replace column references with the wrong values.
Good point, I'll add labels. |
|
TFTR! bors r+ |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 7672be3 to blathers/backport-release-22.1-97554: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
During planning for a render node, the physical planning logic checks whether the last created processor rendered its own expressions, adds its render expressions to the processor if not. The check is necessary to handle the case when expressions in a parent render reference those from a child render.
However, render nodes also handle projecting input columns: both removing them and reordering them. Correct planning of the processors depends on the columns at each step being in a particular order. Since physical planning didn't check whether the existing processor projected its columns in some way, the parent render could be combined with its child. This could lead to panics in the parent render expressions when the columns weren't projected as expected. This patch fixes the issue by adding the projection check before allowing the render planning code to add to the previous processor.
Fixes #95633
Release note (bug fix): Fixed a rare bug existing since before 22.1 that
could cause a projected expression to replace column references with the
wrong values.