opt: remove panic when in between filters are not constrained#81465
opt: remove panic when in between filters are not constrained#81465craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
msirek
left a comment
There was a problem hiding this comment.
Nice fix! Another option would have been to keep the panic if the ordering of the in-between filter column matches that of the previous column, but maybe it's better to remove the panic entirely, as you've done. A sub-optimal access path shouldn't necessarily throw an error.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @rharding6373 and @rytaft)
-- commits line 12 at r1:
nit: abondons -> abandons
235bb73 to
62f9fab
Compare
rharding6373
left a comment
There was a problem hiding this comment.
TFTR!
I tried to do that approach at first, but I don't think we have the partition columns readily available (only partitions, whose lengths are unreliable) in our index interface so it would have required additional plumbing for this one case. I think it's better for the optimizer to not crash anyway since failing to find a constraint here does not mean that other parts of the database will fail.
Reviewable status:
Previously, msirek (Mark Sirek) wrote…
nit: abondons -> abandons
Done.
msirek
left a comment
There was a problem hiding this comment.
👍
Reviewable status:
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:
pkg/sql/opt/xform/select_funcs.go line 267 at r2 (raw file):
// // However, since region and zone are in ascending and descending order, // respectively, the optimizer is currently unable to represent a span with
nit: we could represent a span -- I'd say "is unable to build an expression that would correspond to the in-between spans"
Before this change, the optimizer assumed that if partition filters are constrained on an index then the in-between filters are also constrained, and would panic if it was unable to find a constraint. However, when an index is partitioned on multiple columns and the columns have different orders, the optimizer may be unable to generate a constraint for the in between filters of the partitions, even if the partition filters are constrained. This change removes the panic, since the assumption does not hold. As a consequence, the optimizer abandons the GenerateConstrainedScan rule, but doesn't crash. I did not include a release note since this appears to be a rare bug that we have not encountered in the wild. Fixes cockroachdb#80820 Release note: None
62f9fab to
cdbc442
Compare
rharding6373
left a comment
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/sql/opt/xform/select_funcs.go line 267 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
nit: we could represent a span -- I'd say "is unable to build an expression that would correspond to the in-between spans"
Done.
|
Build succeeded: |
Before this change, the optimizer assumed that if partition filters are
constrained on an index then the in-between filters are also
constrained, and would panic if it was unable to find a constraint.
However, when an index is partitioned on multiple columns and the
columns have different orders, the optimizer may be unable to generate a
constraint for the in between filters of the partitions, even if the
partition filters are constrained. This change removes the panic, since
the assumption does not hold. As a consequence, the optimizer abandons
the GenerateConstrainedScan rule, but doesn't crash.
I did not include a release note since this appears to be a rare bug
that we have not encountered in the wild.
Fixes #80820
Release note: None