diff --git a/src/dbnode/storage/index/fields_terms_iterator_test.go b/src/dbnode/storage/index/fields_terms_iterator_test.go index a20d092519..2000bed694 100644 --- a/src/dbnode/storage/index/fields_terms_iterator_test.go +++ b/src/dbnode/storage/index/fields_terms_iterator_test.go @@ -356,6 +356,10 @@ type stubTermIterator struct { points []iterpoint } +func (s *stubTermIterator) Empty() bool { + return len(s.points) == 0 +} + func (s *stubTermIterator) Next() bool { if len(s.points) == 0 { return false diff --git a/src/m3ninx/index/segment/builder/multi_segments_field_iter.go b/src/m3ninx/index/segment/builder/multi_segments_field_iter.go index 8625fa93bb..4f21f35858 100644 --- a/src/m3ninx/index/segment/builder/multi_segments_field_iter.go +++ b/src/m3ninx/index/segment/builder/multi_segments_field_iter.go @@ -38,7 +38,7 @@ func newFieldIterFromSegments( if err != nil { return nil, err } - if !iter.Next() { + if iter.Empty() { // Don't consume this iterator if no results. if err := xerrors.FirstError(iter.Err(), iter.Close()); err != nil { return nil, err diff --git a/src/m3ninx/index/segment/builder/multi_segments_field_postings_list_iter.go b/src/m3ninx/index/segment/builder/multi_segments_field_postings_list_iter.go index 08517b224f..08a3c5151c 100644 --- a/src/m3ninx/index/segment/builder/multi_segments_field_postings_list_iter.go +++ b/src/m3ninx/index/segment/builder/multi_segments_field_postings_list_iter.go @@ -38,7 +38,7 @@ func newFieldPostingsListIterFromSegments( if err != nil { return nil, err } - if !iter.Next() { + if iter.Empty() { // Don't consume this iterator if no results. if err := xerrors.FirstError(iter.Err(), iter.Close()); err != nil { return nil, err diff --git a/src/m3ninx/index/segment/builder/multi_segments_field_postings_list_iter_test.go b/src/m3ninx/index/segment/builder/multi_segments_field_postings_list_iter_test.go index 9e781d3815..5d49119746 100644 --- a/src/m3ninx/index/segment/builder/multi_segments_field_postings_list_iter_test.go +++ b/src/m3ninx/index/segment/builder/multi_segments_field_postings_list_iter_test.go @@ -100,7 +100,11 @@ func TestFieldPostingsListIterFromSegments(t *testing.T) { Fields: []doc.Field{ {Name: []byte("delta"), Value: []byte("22")}, {Name: []byte("gamma"), Value: []byte("33")}, - {Name: []byte("theta"), Value: []byte("44")}, + // Test field that alphanumerically precedes the specialized + // IDReservedFieldName which is prefixed with _. The iterators + // here sort fields and so we want to make sure fields that + // precede that special field still work properly. + {Name: []byte("__snowflake"), Value: []byte("44")}, }, }, }), @@ -113,18 +117,57 @@ func TestFieldPostingsListIterFromSegments(t *testing.T) { require.NoError(t, builder.AddSegments(segments)) iter, err := b.FieldsPostingsList() require.NoError(t, err) - // Perform both present/not present checks per field/field postings list. + + // Confirm all posting list fields are present in docs. for iter.Next() { field, pl := iter.Current() docIter, err := b.AllDocs() require.NoError(t, err) for docIter.Next() { - doc := docIter.Current() + d := docIter.Current() pID := docIter.PostingsID() - found := checkIfFieldExistsInDoc(field, doc) - require.Equal(t, found, pl.Contains(pID)) + found := checkIfFieldExistsInDoc(field, d) + + // Special case ID field which is present in postings list + // for all indexes but not part of the doc itself. + if bytes.Equal(field, doc.IDReservedFieldName) { + require.Equal(t, found, false) + require.Equal(t, pl.Contains(pID), true) + } else { + require.Equal(t, found, pl.Contains(pID)) + } + } + require.NoError(t, docIter.Err()) + require.NoError(t, docIter.Close()) + } + require.NoError(t, iter.Err()) + require.NoError(t, iter.Close()) + + // Confirm all docs' fields are present in postings list. + docIter, err := b.AllDocs() + require.NoError(t, err) + for docIter.Next() { + doc := docIter.Current() + + for _, f := range doc.Fields { + iter, err := b.FieldsPostingsList() + require.NoError(t, err) + + present := false + for iter.Next() { + field, _ := iter.Current() + present = present || bytes.Equal(f.Name, field) + if present { + break + } + } + require.True(t, present) + require.NoError(t, iter.Err()) + require.NoError(t, iter.Close()) } } + require.NoError(t, docIter.Err()) + require.NoError(t, docIter.Close()) } func checkIfFieldExistsInDoc( diff --git a/src/m3ninx/index/segment/builder/multi_segments_multi_key_iter.go b/src/m3ninx/index/segment/builder/multi_segments_multi_key_iter.go index ef906244fb..81c0cc66a4 100644 --- a/src/m3ninx/index/segment/builder/multi_segments_multi_key_iter.go +++ b/src/m3ninx/index/segment/builder/multi_segments_multi_key_iter.go @@ -36,7 +36,6 @@ type keyIterator interface { var _ keyIterator = &multiKeyIterator{} type multiKeyIterator struct { - firstNext bool closeIters []keyIterator iters []keyIterator currIters []keyIterator @@ -49,8 +48,6 @@ func newMultiKeyIterator() *multiKeyIterator { } func (i *multiKeyIterator) reset() { - i.firstNext = true - for j := range i.closeIters { i.closeIters[j] = nil } @@ -73,16 +70,18 @@ func (i *multiKeyIterator) add(iter keyIterator) { i.tryAddCurr(iter) } +func (i *multiKeyIterator) Empty() bool { + // Use i.closeIters to indicate if any iters were added instead of + // i.iters or i.currIter since those are popped off during iteration, + // whereas i.closeIters are only removed on reset. + return len(i.closeIters) == 0 +} + func (i *multiKeyIterator) Next() bool { if len(i.iters) == 0 { return false } - if i.firstNext { - i.firstNext = false - return true - } - for _, currIter := range i.currIters { currNext := currIter.Next() if currNext { diff --git a/src/m3ninx/index/segment/builder/multi_segments_terms_iter.go b/src/m3ninx/index/segment/builder/multi_segments_terms_iter.go index 3be2497931..e15a4d484e 100644 --- a/src/m3ninx/index/segment/builder/multi_segments_terms_iter.go +++ b/src/m3ninx/index/segment/builder/multi_segments_terms_iter.go @@ -103,7 +103,7 @@ func (i *termsIterFromSegments) setField(field []byte) error { if err != nil { return err } - if !iter.Next() { + if iter.Empty() { // Don't consume this iterator if no results if err := xerrors.FirstError(iter.Err(), iter.Close()); err != nil { return err @@ -120,6 +120,10 @@ func (i *termsIterFromSegments) setField(field []byte) error { return nil } +func (i *termsIterFromSegments) Empty() bool { + return i.keyIter.Empty() +} + func (i *termsIterFromSegments) Next() bool { for { if i.err != nil { diff --git a/src/m3ninx/index/segment/builder/terms_iter.go b/src/m3ninx/index/segment/builder/terms_iter.go index 40140359f2..e77e7e9659 100644 --- a/src/m3ninx/index/segment/builder/terms_iter.go +++ b/src/m3ninx/index/segment/builder/terms_iter.go @@ -45,6 +45,10 @@ func newTermsIter( } } +func (b *termsIter) Empty() bool { + return b.Len() == 0 +} + func (b *termsIter) Next() bool { if b.done || b.err != nil { return false diff --git a/src/m3ninx/index/segment/empty.go b/src/m3ninx/index/segment/empty.go index 45b920781a..399da315e3 100644 --- a/src/m3ninx/index/segment/empty.go +++ b/src/m3ninx/index/segment/empty.go @@ -27,6 +27,7 @@ var EmptyOrderedBytesIterator OrderedBytesIterator = emptyBytesIter{} type emptyBytesIter struct{} +func (e emptyBytesIter) Empty() bool { return true } func (e emptyBytesIter) Next() bool { return false } func (e emptyBytesIter) Current() []byte { return nil } func (e emptyBytesIter) Err() error { return nil } @@ -38,6 +39,7 @@ var EmptyTermsIterator TermsIterator = emptyTermsIter{} type emptyTermsIter struct{} +func (e emptyTermsIter) Empty() bool { return true } func (e emptyTermsIter) Next() bool { return false } func (e emptyTermsIter) Current() ([]byte, postings.List) { return nil, nil } func (e emptyTermsIter) Err() error { return nil } diff --git a/src/m3ninx/index/segment/fst/fst_terms_iterator.go b/src/m3ninx/index/segment/fst/fst_terms_iterator.go index 734ebbd911..091016af91 100644 --- a/src/m3ninx/index/segment/fst/fst_terms_iterator.go +++ b/src/m3ninx/index/segment/fst/fst_terms_iterator.go @@ -114,6 +114,10 @@ func (f *fstTermsIter) CurrentOffset() uint64 { return f.currentValue } +func (f *fstTermsIter) Empty() bool { + return f.Len() == 0 +} + func (f *fstTermsIter) Current() []byte { return f.current } diff --git a/src/m3ninx/index/segment/fst/fst_terms_postings_iterator.go b/src/m3ninx/index/segment/fst/fst_terms_postings_iterator.go index b01e998858..28c9d8f5d3 100644 --- a/src/m3ninx/index/segment/fst/fst_terms_postings_iterator.go +++ b/src/m3ninx/index/segment/fst/fst_terms_postings_iterator.go @@ -79,6 +79,10 @@ func (f *fstTermsPostingsIter) reset( f.termsIter = termsIter } +func (f *fstTermsPostingsIter) Empty() bool { + return f.termsIter.Empty() +} + func (f *fstTermsPostingsIter) Next() bool { if f.err != nil { return false diff --git a/src/m3ninx/index/segment/mem/bytes_slice_iterator.go b/src/m3ninx/index/segment/mem/bytes_slice_iterator.go index a6f6e21974..5f2c51054a 100644 --- a/src/m3ninx/index/segment/mem/bytes_slice_iterator.go +++ b/src/m3ninx/index/segment/mem/bytes_slice_iterator.go @@ -48,6 +48,10 @@ func newBytesSliceIter(slice [][]byte, opts Options) *bytesSliceIter { } } +func (b *bytesSliceIter) Empty() bool { + return len(b.backingSlice) == 0 +} + func (b *bytesSliceIter) Next() bool { if b.done || b.err != nil { return false diff --git a/src/m3ninx/index/segment/mem/terms_iterator.go b/src/m3ninx/index/segment/mem/terms_iterator.go index cb4eca8cb6..cd051c69ac 100644 --- a/src/m3ninx/index/segment/mem/terms_iterator.go +++ b/src/m3ninx/index/segment/mem/terms_iterator.go @@ -90,6 +90,10 @@ func (b *termsIter) Len() int { return len(b.backingSlice) } +func (b *termsIter) Empty() bool { + return len(b.backingSlice) == 0 +} + func (b *termsIter) Close() error { b.current = nil b.opts.BytesSliceArrayPool().Put(b.backingSlice) diff --git a/src/m3ninx/index/segment/segment_mock.go b/src/m3ninx/index/segment/segment_mock.go index dcb5f8e5f5..341f8a3a3d 100644 --- a/src/m3ninx/index/segment/segment_mock.go +++ b/src/m3ninx/index/segment/segment_mock.go @@ -714,6 +714,20 @@ func (mr *MockFieldsIteratorMockRecorder) Current() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Current", reflect.TypeOf((*MockFieldsIterator)(nil).Current)) } +// Empty mocks base method. +func (m *MockFieldsIterator) Empty() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Empty") + ret0, _ := ret[0].(bool) + return ret0 +} + +// Empty indicates an expected call of Empty. +func (mr *MockFieldsIteratorMockRecorder) Empty() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Empty", reflect.TypeOf((*MockFieldsIterator)(nil).Empty)) +} + // Err mocks base method. func (m *MockFieldsIterator) Err() error { m.ctrl.T.Helper() @@ -794,6 +808,20 @@ func (mr *MockTermsIteratorMockRecorder) Current() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Current", reflect.TypeOf((*MockTermsIterator)(nil).Current)) } +// Empty mocks base method. +func (m *MockTermsIterator) Empty() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Empty") + ret0, _ := ret[0].(bool) + return ret0 +} + +// Empty indicates an expected call of Empty. +func (mr *MockTermsIteratorMockRecorder) Empty() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Empty", reflect.TypeOf((*MockTermsIterator)(nil).Empty)) +} + // Err mocks base method. func (m *MockTermsIterator) Err() error { m.ctrl.T.Helper() diff --git a/src/m3ninx/index/segment/types.go b/src/m3ninx/index/segment/types.go index 7a265175c2..b20cf04e2c 100644 --- a/src/m3ninx/index/segment/types.go +++ b/src/m3ninx/index/segment/types.go @@ -132,6 +132,9 @@ type FieldsIterator interface { // Current returns the current field. // NB: the field returned is only valid until the subsequent call to Next(). Current() []byte + + // Empty returns true if there are no fields in the iterator. + Empty() bool } // TermsIterator iterates over all known terms for the provided field. @@ -141,6 +144,9 @@ type TermsIterator interface { // Current returns the current element. // NB: the element returned is only valid until the subsequent call to Next(). Current() (term []byte, postings postings.List) + + // Empty returns true if there are no terms. + Empty() bool } // Iterator holds common iterator methods.