Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dbnode] Fix multi-segment field iterator support of __ prefix fields alphanumerically before __m3ninx_id #4095

Merged
merged 3 commits into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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