fix: Handle compound filters on related indexed fields #2575
Conversation
The current behaviour is documented in this commit
Contains two changes: - complex (relation) filters in indexes are not passed on to the index fetcher - it cannot handle them at the moment - Adds support for one-many joins from the secondary side in the invertableJoin type, otherwise it fails to yield multiple results
AndrewSisley
added
bug
planner/filter/complex.go
Outdated
| @@ -33,7 +33,7 @@ func isComplex(conditions any, seekRelation bool) bool { | |||
| if op, ok := k.(*mapper.Operator); ok { | |||
| switch op.Operation { | |||
| case request.FilterOpOr, request.FilterOpAnd, request.FilterOpNot: | |||
| if v, ok := v.([]any); ok && len(v) == 1 { | |||
| if v, ok := v.([]any); ok && len(v) == 0 { | |||
There was a problem hiding this comment.
It is not ideal to keep this as-is long-term, as it will prevent a possible performance-shortcut once John's fetcher rework gets merged, but for now it is a simple way of excluding the index fetcher filter's lack of support for compound filters..
There was a problem hiding this comment.
If we are going to do this might as well remove the if block all together. It's never going to be of length 0.
There was a problem hiding this comment.
Will do. I think it is valid to submit an empty and/or clause, but probably not really worth the extra code for that :)
- Remove len 0 check
AndrewSisley
changed the title
planner/type_join.go
Outdated
| @@ -487,6 +486,9 @@ type invertibleTypeJoin struct { | |||
| secondaryFieldIndex immutable.Option[int] | |||
| secondaryFetchLimit uint | |||
|
|
|||
| // docsToYield contains cocuments read and ready to be yielded by this node. | |||
There was a problem hiding this comment.
:) will fix
- typo
| @@ -556,6 +558,17 @@ func (join *invertibleTypeJoin) processSecondResult(secondDocs []core.Doc) (any, | |||
| } | |||
|
|
|||
| func (join *invertibleTypeJoin) Next() (bool, error) { | |||
There was a problem hiding this comment.
thought: This function is really difficult to understand.
There was a problem hiding this comment.
Yes, and IMO the variable names don't really pair up well either, and they actually represent different things in different situations.
| Request: `query { | ||
| Program( | ||
| filter: { | ||
| _and: [ |
There was a problem hiding this comment.
lol - nice catch, thanks 😁
I'll fix this shortly (new issue/PR)
| Request: `query { | ||
| Program( | ||
| filter: { | ||
| _and: [ |
Relevant issue(s)
Resolves #2572
Description
Handles compound filters targeting related indexed fields, and one-many joins from the many side.
The invertableJoin issue may be affecting non indexed joins.
First commit documents the existing behaviour, second commit fixes it.
There is another issue in this space not solved by this PR: #2574