Skip to content

Commit

Permalink
fix: Add check to filter result for logical ops (#2573)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #2571

## Description

This PR adds a check post doc filtering in the fetcher to see if we need
to rely on the external conditions instead to ensure a valid filtering
operation. This is specially important when filtering with logical
operators `_and` and `_or`.
  • Loading branch information
fredcarle authored May 1, 2024
1 parent 3b90e8b commit 4248007
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 5 deletions.
9 changes: 6 additions & 3 deletions planner/filter/complex.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

// IsComplex returns true if the provided filter is complex.
// A filter is considered complex if it contains a relation
// object withing an _or or _not operator not necessarily being
// object withing an _or, _and or _not operator not necessarily being
// its direct child.
func IsComplex(filter *mapper.Filter) bool {
if filter == nil {
Expand All @@ -31,8 +31,11 @@ func isComplex(conditions any, seekRelation bool) bool {
case map[connor.FilterKey]any:
for k, v := range typedCond {
if op, ok := k.(*mapper.Operator); ok {
if (op.Operation == request.FilterOpOr && len(v.([]any)) > 1) ||
op.Operation == request.FilterOpNot {
switch op.Operation {
case request.FilterOpOr, request.FilterOpAnd, request.FilterOpNot:
if v, ok := v.([]any); ok && len(v) == 1 {
continue
}
if isComplex(v, true) {
return true
}
Expand Down
2 changes: 1 addition & 1 deletion planner/filter/complex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestIsComplex(t *testing.T) {
m("published", m("rating", m("_gt", 4.0))),
),
),
isComplex: false,
isComplex: true,
},
{
name: "relation inside _and and _or",
Expand Down
2 changes: 1 addition & 1 deletion planner/type_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func prepareScanNodeFilterForTypeJoin(
parent.filter.Conditions = filter.Merge(
parent.filter.Conditions, scan.filter.Conditions)
}
filter.RemoveField(scan.filter, subType.Field)
scan.filter = nil
} else {
var parentFilter *mapper.Filter
scan.filter, parentFilter = filter.SplitByFields(scan.filter, subType.Field)
Expand Down
127 changes: 127 additions & 0 deletions tests/integration/query/one_to_one/with_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,3 +492,130 @@ func TestQueryOneToOneWithCompoundOrFilterThatIncludesRelation(t *testing.T) {

testUtils.ExecuteTestCase(t, test)
}

func TestQueryOneToOne_WithCompoundFiltersThatIncludesRelation_ShouldReturnResults(t *testing.T) {
test := testUtils.TestCase{
Description: "One-to-one relation with _and filter that includes relation",
Actions: []any{
testUtils.SchemaUpdate{
Schema: bookAuthorGQLSchema,
},
testUtils.CreateDoc{
CollectionID: 0,
// bae-fd541c25-229e-5280-b44b-e5c2af3e374d
Doc: `{
"name": "Painted House",
"rating": 4.9
}`,
},
testUtils.CreateDoc{
CollectionID: 0,
// bae-437092f3-7817-555c-bf8a-cc1c5a0a0db6
Doc: `{
"name": "Some Book",
"rating": 4.0
}`,
},
testUtils.CreateDoc{
CollectionID: 0,
// bae-66ba0c48-4984-5b44-83dd-edb791a54b7d
Doc: `{
"name": "Some Other Book",
"rating": 3.0
}`,
},
testUtils.CreateDoc{
CollectionID: 1,
// bae-3bfe0092-e31f-5ebe-a3ba-fa18fac448a6
Doc: `{
"name": "John Grisham",
"age": 65,
"verified": true,
"published_id": "bae-fd541c25-229e-5280-b44b-e5c2af3e374d"
}`,
},
testUtils.CreateDoc{
CollectionID: 1,
// bae-5dac8488-0f75-5ddf-b08b-804b3d33a239
Doc: `{
"name": "Some Writer",
"age": 45,
"verified": false,
"published_id": "bae-437092f3-7817-555c-bf8a-cc1c5a0a0db6"
}`,
},
testUtils.CreateDoc{
CollectionID: 1,
// bae-8b0c345b-dda7-573c-b5f1-5fa1d70593e1
Doc: `{
"name": "Some Other Writer",
"age": 30,
"verified": true,
"published_id": "bae-66ba0c48-4984-5b44-83dd-edb791a54b7d"
}`,
},
testUtils.Request{
Request: `query {
Book(filter: {_or: [
{rating: {_gt: 4.0}},
{author: {age: {_eq: 30}}}
]}) {
name
rating
}
}`,
Results: []map[string]any{
{
"name": "Some Other Book",
"rating": 3.0,
},
{
"name": "Painted House",
"rating": 4.9,
},
},
},
testUtils.Request{
Request: `query {
Book(filter: {_and: [
{rating: {_ge: 4.0}},
{author: {age: {_eq: 45}}}
]}) {
name
rating
}
}`,
Results: []map[string]any{
{
"name": "Some Book",
"rating": 4.0,
},
},
},
testUtils.Request{
// This is the same as {_not: {_and: [{rating: {_ge: 4.0}}, {author: {age: {_eq: 45}}}]}}
Request: `query {
Book(filter: {_not: {
rating: {_ge: 4.0},
author: {age: {_eq: 45}}
}}) {
name
rating
}
}`,
Results: []map[string]any{
{
"name": "Some Other Book",
"rating": 3.0,
},
{
"name": "Painted House",
"rating": 4.9,
},
},
},
},
}

testUtils.ExecuteTestCase(t, test)
}

0 comments on commit 4248007

Please sign in to comment.