Skip to content

Commit

Permalink
fix: No panic if filter condition on indexed field is empty (#2929)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #2916

## Description

Check if given filter condition is empty before proceeding with
secondary index fetching.
  • Loading branch information
islamaliev committed Aug 20, 2024
1 parent defb37a commit 9422244
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 13 deletions.
30 changes: 17 additions & 13 deletions internal/db/fetcher/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ type IndexFetcher struct {
col client.Collection
txn datastore.Txn
indexFilter *mapper.Filter
docFilter *mapper.Filter
doc *encodedDocument
mapping *core.DocumentMapping
indexedFields []client.FieldDefinition
Expand Down Expand Up @@ -71,7 +70,6 @@ func (f *IndexFetcher) Init(
f.resetState()

f.col = col
f.docFilter = filter
f.doc = &encodedDocument{}
f.mapping = docMapper
f.txn = txn
Expand Down Expand Up @@ -100,15 +98,20 @@ outer:
}
f.indexIter = iter

if f.docFetcher != nil && len(f.docFields) > 0 {
// if it turns out that we can't use the index, we need to fall back to the document fetcher
if f.indexIter == nil {
f.docFields = fields
}

if len(f.docFields) > 0 {
err = f.docFetcher.Init(
ctx,
identity,
f.txn,
acp,
f.col,
f.docFields,
f.docFilter,
filter,
f.mapping,
false,
false,
Expand All @@ -119,14 +122,16 @@ outer:
}

func (f *IndexFetcher) Start(ctx context.Context, spans core.Spans) error {
err := f.indexIter.Init(ctx, f.txn.Datastore())
if err != nil {
return err
if f.indexIter == nil {
return f.docFetcher.Start(ctx, spans)
}
return nil
return f.indexIter.Init(ctx, f.txn.Datastore())
}

func (f *IndexFetcher) FetchNext(ctx context.Context) (EncodedDocument, ExecInfo, error) {
if f.indexIter == nil {
return f.docFetcher.FetchNext(ctx)
}
totalExecInfo := f.execInfo
defer func() { f.execInfo.Add(totalExecInfo) }()
f.execInfo.Reset()
Expand Down Expand Up @@ -176,7 +181,7 @@ func (f *IndexFetcher) FetchNext(ctx context.Context) (EncodedDocument, ExecInfo
}
}

if f.docFetcher != nil && len(f.docFields) > 0 {
if len(f.docFields) > 0 {
targetKey := base.MakeDataStoreKeyWithCollectionAndDocID(f.col.Description(), string(f.doc.id))
spans := core.NewSpans(core.NewSpan(targetKey, targetKey.PrefixEnd()))
err := f.docFetcher.Start(ctx, spans)
Expand Down Expand Up @@ -204,10 +209,10 @@ func (f *IndexFetcher) FetchNext(ctx context.Context) (EncodedDocument, ExecInfo
}

func (f *IndexFetcher) Close() error {
if f.indexIter != nil {
return f.indexIter.Close()
if f.indexIter == nil {
return f.docFetcher.Close()
}
return nil
return f.indexIter.Close()
}

// resetState resets the mutable state of this IndexFetcher, returning the state to how it
Expand All @@ -217,7 +222,6 @@ func (f *IndexFetcher) resetState() {

f.col = nil
f.txn = nil
f.docFilter = nil
f.doc = nil
f.mapping = nil
f.indexedFields = nil
Expand Down
5 changes: 5 additions & 0 deletions internal/db/fetcher/indexer_iterators.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,11 @@ func (f *IndexFetcher) createIndexIterator() (indexIterator, error) {
return nil, err
}

// this can happen if a query contains an empty condition like User(filter: {name: {}})
if len(fieldConditions) == 0 {
return nil, nil
}

matchers, err := createValueMatchers(fieldConditions)
if err != nil {
return nil, err
Expand Down
41 changes: 41 additions & 0 deletions tests/integration/index/query_with_index_only_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -677,3 +677,44 @@ func TestQueryWithIndex_WithNotLikeFilter_ShouldFetch(t *testing.T) {

testUtils.ExecuteTestCase(t, test)
}

func TestQueryWithIndex_EmptyFilterOnIndexedField_ShouldSucceed(t *testing.T) {
test := testUtils.TestCase{
Actions: []any{
testUtils.SchemaUpdate{
Schema: `
type User {
name: String @index
age: Int
}`,
},
testUtils.CreateDoc{
Doc: `{
"name": "Islam",
"age": 33
}`,
},
testUtils.CreateDoc{
Doc: `{
"name": "John",
"age": 21
}`,
},
testUtils.Request{
Request: `query {
User(filter: {name: {}}) {
name
}
}`,
Results: map[string]any{
"User": []map[string]any{
{"name": "Islam"},
{"name": "John"},
},
},
},
},
}

testUtils.ExecuteTestCase(t, test)
}

0 comments on commit 9422244

Please sign in to comment.