-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-24.1: opt: don't apply huge cost for full scans of virtual tables #123893
release-24.1: opt: don't apply huge cost for full scans of virtual tables #123893
Conversation
This commit fixes - what I believe is - an oversight where we applied the huge cost to the plans with full scans of virtual tables when `disallow_full_table_scans` variable is set. The intent behind this penalty is to make plans with full scans prohibitively expensive so that other plans (without full scans) would be preferred, and if there is no other plan, then the query would error out. Implementation of this approach was incomplete for virtual tables: namely, we'd apply the cost penalty, but then we would not actually error out the query. Additionally, given that we don't have stats on virtual tables, we would apply the cost penalty to all full scans of virtual tables. As a result, we would either pick a plan without the full scan (that might actually be worse) or would allow the plan with the full scan to get executed still. We would also favor plans with fewer full scans of virtual tables (which might also be worse than had we not applied the cost penalty). This commit adjusts the code to not apply the cost penalty to full scans of virtual tables. This change is gated based on the new session variable `optimizer_apply_full_scan_penalty_to_virtual_tables` which defaults to `false` on master. We can discuss on the backport PRs whether it should be default to `true` there to require users to opt in for this PR to have an effect. Release note: None
e0a3b56
to
99caaa8
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
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 17 of 17 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @michae2, @rytaft, and @yuzefovich)
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 17 of 17 files at r1, all commit messages.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @mgartner, @rytaft, and @yuzefovich)
Backport 1/1 commits from #123780 on behalf of @yuzefovich.
/cc @cockroachdb/release
This commit fixes - what I believe is - an oversight where we applied the huge cost to the plans with full scans of virtual tables when
disallow_full_table_scans
variable is set. The intent behind this penalty is to make plans with full scans prohibitively expensive so that other plans (without full scans) would be preferred, and if there is no other plan, then the query would error out. Implementation of this approach was incomplete for virtual tables: namely, we'd apply the cost penalty, but then we would not actually error out the query. Additionally, given that we don't have stats on virtual tables, we would apply the cost penalty to all full scans of virtual tables.As a result, we would either pick a plan without the full scan (that might actually be worse) or would allow the plan with the full scan to get executed still. We would also favor plans with fewer full scans of virtual tables (which might also be worse than had we not applied the cost penalty). This commit adjusts the code to not apply the cost penalty to full scans of virtual tables.
This change is gated based on the new session variable
optimizer_apply_full_scan_penalty_to_virtual_tables
which defaults tofalse
on master. We can discuss on the backport PRs whether it should be default totrue
there to require users to opt in for this PR to have an effect.Fixes: #122708.
Release note: None
Release justification: bug fix.