Skip to content

Commit

Permalink
[dbnode] Fix multi-segment field iterator support of __ prefix fields…
Browse files Browse the repository at this point in the history
… alphanumerically before __m3ninx_id (#4095)
  • Loading branch information
robskillington authored Apr 4, 2022
1 parent cf6d87a commit 68be4dd
Show file tree
Hide file tree
Showing 14 changed files with 122 additions and 16 deletions.
4 changes: 4 additions & 0 deletions src/dbnode/storage/index/fields_terms_iterator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")},
},
},
}),
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type keyIterator interface {
var _ keyIterator = &multiKeyIterator{}

type multiKeyIterator struct {
firstNext bool
closeIters []keyIterator
iters []keyIterator
currIters []keyIterator
Expand All @@ -49,8 +48,6 @@ func newMultiKeyIterator() *multiKeyIterator {
}

func (i *multiKeyIterator) reset() {
i.firstNext = true

for j := range i.closeIters {
i.closeIters[j] = nil
}
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions src/m3ninx/index/segment/builder/terms_iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/m3ninx/index/segment/empty.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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 }
Expand Down
4 changes: 4 additions & 0 deletions src/m3ninx/index/segment/fst/fst_terms_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions src/m3ninx/index/segment/fst/fst_terms_postings_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/m3ninx/index/segment/mem/bytes_slice_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/m3ninx/index/segment/mem/terms_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 28 additions & 0 deletions src/m3ninx/index/segment/segment_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/m3ninx/index/segment/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down

0 comments on commit 68be4dd

Please sign in to comment.