feat(qp): prune branches that cannot lead to the subject type of the …#2968
feat(qp): prune branches that cannot lead to the subject type of the …#2968miparnisari merged 3 commits intomainfrom
Conversation
github-actions
bot
added
area/api v1
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (74.84%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2968 +/- ##
==========================================
+ Coverage 74.82% 74.84% +0.02%
==========================================
Files 497 498 +1
Lines 60656 60736 +80
==========================================
+ Hits 45382 45451 +69
- Misses 12115 12123 +8
- Partials 3159 3162 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
miparnisari
force-pushed
the
qp-subject-type-shear
branch
from
9779e99 to
16bdda5
Compare
miparnisari
force-pushed
the
qp-subject-type-shear
branch
from
16bdda5 to
e733393
Compare
barakmich
left a comment
barakmich
left a comment
There was a problem hiding this comment.
So I think this could be simplified, namely, creating a closure that understands the targetSubjectType, calls MutateOutline (which walks bottom up https://github.com/authzed/spicedb/blob/main/pkg/query/mutations.go#L9) -- though MutateOutline might need to become a little more aware of what happens if the function it's passed returns a null type -- and then simply return a null if it's a datastore iterator and the types don't match.
The tests however should totally still work 🙂
| return nil, ps.rewriteError(ctx, err) | ||
| } | ||
|
|
||
| optimized = queryopt.ApplyReachabilityPruning(optimized, req.Subject.Object.ObjectType) |
There was a problem hiding this comment.
We should make this a little more generalized (ie, this is the first request-parameterized optimization!) but this is fine for now.
| func ApplyReachabilityPruning(co query.CanonicalOutline, targetSubjectType string) query.CanonicalOutline { | ||
| pruned, _ := pruneOutline(co.Root, targetSubjectType) | ||
|
|
||
| // TODO do i need to call FillMissingIds? |
There was a problem hiding this comment.
Not if it's just pruning 🙂
miparnisari
force-pushed
the
qp-subject-type-shear
branch
from
ec093ce to
17b9ff3
Compare
barakmich
left a comment
barakmich
left a comment
There was a problem hiding this comment.
Approved, with two small changes
| // their existing IDs. Newly synthesized nodes (ID==0) receive fresh IDs via | ||
| // query.FillMissingNodeIDs. The returned CanonicalOutline is ready to compile. | ||
| func ApplyOptimizations(co query.CanonicalOutline, names []string) (query.CanonicalOutline, error) { | ||
| func ApplyOptimizations(co query.CanonicalOutline, names []string, requestParams RequestParams) (query.CanonicalOutline, error) { |
| // NewTransform creates a whole-tree transformation for this optimizer, | ||
| // optionally using request-specific parameters. Each transform typically | ||
| // calls query.MutateOutline internally with its own mutations. | ||
| NewTransform func(RequestParams) OutlineTransform |
There was a problem hiding this comment.
I would do one little trick here, which is to name this signature (eg, RequestOutlineTransform, under the above) and have a simple identity helper that wraps it (turning the caveat pushdown into something like NewTransform: withoutRequestParams(caveatPushdown))
2 participants
…request
Description
This is an optimization for Check API when using the query planner. We prune branches of the tree that can never lead to the subject type in the request.
Testing
Unit tests
References