-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
opt: fix incorrect results returned by non-recursive WITH RECURSIVE CTE #98042
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! 2 of 0 LGTMs obtained (waiting on @RaduBerinde)
Prior to this commit, queries that used a CTE marked as WITH RECURSIVE which did not reference itself (i.e., it was not actually recursive) could return incorrect results. This was because the UNION ALL in the CTE was incorrectly converted to a UNION by the optimizer. This commit fixes the problem by ensuring that UNION ALL is used if the original CTE contained UNION ALL. Fixes cockroachdb#93370 Release note (bug fix): Fixed a bug in which CTEs marked as WITH RECURSIVE which were not actually recursive could return incorrect results. This could happen if the CTE used a UNION ALL, because the optimizer incorrectly converted the UNION ALL to a UNION. This bug has existed since suppport for recursive CTEs was first added in v20.1, and has now been fixed.
4bc9330
to
74fa893
Compare
Oopsie.. LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFTRs!
bors r+
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @DrewKimball, @mgartner, and @RaduBerinde)
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 74fa893 to blathers/backport-release-22.1-98042: 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. error creating merge commit from 74fa893 to blathers/backport-release-22.2-98042: 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.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Prior to this commit, queries that used a CTE marked as
WITH RECURSIVE
which did not reference itself (i.e., it was not actually recursive) could return incorrect results. This was because theUNION ALL
in the CTE was incorrectly converted to aUNION
by the optimizer. This commit fixes the problem by ensuring thatUNION ALL
is used if the original CTE containedUNION ALL
.Fixes #93370
Release note (bug fix): Fixed a bug in which CTEs marked as
WITH RECURSIVE
which were not actually recursive could return incorrect results. This could happen if the CTE used aUNION ALL
, because the optimizer incorrectly converted theUNION ALL
to aUNION
. This bug has existed since suppport for recursive CTEs was first added in v20.1, and has now been fixed.