sql/schemachanger/scplan/internal/rules: rework dep rules #82256
Conversation
|
This change is |
ajwerner
force-pushed
the
ajwerner/rework-dep-rules
branch
from
05e335b to
ad72e44
Compare
| joinOnIndexID(from, to), | ||
| targetStatusEq(fromTarget, toTarget, scpb.ToAbsent), | ||
| currentStatusEq(fromNode, toNode, scpb.Status_ABSENT), | ||
| rel.Filter("secondaryIndexPartialIsNotBeingDropped", from)(func( |
There was a problem hiding this comment.
Kebab-case all the rel string literals?
There was a problem hiding this comment.
I made the filter functions camelCase. I'm starting to dislike the kebabs. If I were typing this language I would not be pleased with kabobs I don't think. Sometimes I think about what an interpreter would look like. I'm going to keep iterating around these parts, happy to make further changes later.
| func(from, fromTarget, fromNode, to, toTarget, toNode rel.Var) rel.Clauses { | ||
| return rel.Clauses{ | ||
| from.Type((*scpb.TemporaryIndex)(nil)), | ||
| to.Type((*scpb.PrimaryIndex)(nil), (*scpb.SecondaryIndex)(nil)), |
There was a problem hiding this comment.
I wish there was a way to make these type unions pop out more systematically somehow. It's why I originally newlined each subtype, I found that it helped readability. I think this is fine though.
ajwerner
force-pushed
the
ajwerner/rework-dep-rules
branch
from
ad72e44 to
2aaa4fe
Compare
|
both the builds are certified flakes bors r+ |
|
Build failed: |
|
bors r+ |
|
Build failed: |
Before this change, an index which had it conditions met could be used even if the input query did not constrain the index at all. We never want this to be the case for queries we use; we should always be able to constrain the database at least a little. This rule caught a bug that made logic tests slow. I think it'll be very handy. Note that as a special case, an index which indexes not columns permits any scan. This is useful for testing the logic of the library. Release note: None
ajwerner
force-pushed
the
ajwerner/rework-dep-rules
branch
from
2aaa4fe to
9874591
Compare
The dep rules helpers were brittle. Instead, make it easier to construct rel queries directly. Release note: None
ajwerner
force-pushed
the
ajwerner/rework-dep-rules
branch
from
9874591 to
e5ee890
Compare
|
@postamar I added a new first commit to this which, in retrospect, is an important one. There was a buglet where we'd say that an index can be used even if it matches none of the attributes. This lead to the possibility of writing rules which would not be index accelerated. The hope is to always index accelerate rule evaluation. The immediately caught the bug which lead to the change making logic tests slow. PTAL before I bors it. |
|
I'm going to bors it. bors r+ |
|
Build succeeded: |
The dep rules helpers were brittle. Instead, make it easier to construct rel
queries directly.
Release note: None