Skip to content

Commit

Permalink
quick fix sort issue (#87)
Browse files Browse the repository at this point in the history
* quick fix to sort issue

It has been observed that in some cases the computed sort value
for a DocumentMatch becomes corrupted.  The problem has been
traced back to the doc values uncompressed slices, but for now
a quick fix is proposed to copy the bytes associate with the
sort key, ensuring that no other doc values operations can
corrupt them.
  • Loading branch information
mschoch authored Jan 4, 2022
1 parent 7fbf27b commit f34c4ac
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
80 changes: 80 additions & 0 deletions index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1738,3 +1738,83 @@ func TestCrudWithNoMMap(t *testing.T) {
}
}
}

// TestBug87 reproduces a situation in which a search matches several documents
// and we compare the document's stored value for the _id field, with the
// document's sort value. The sort value should be the same _id, but
// comes from the doc values storage.
// In this case, because doc values were loaded from multiple chunks, an
// "uncompressed" buffer is reused. Incorrect use of of these doc values
// bytes in computed sort values may lead to incorrect sort order and other
// undesired behavior.
func TestBug87(t *testing.T) {
tmpIndexPath := createTmpIndexPath(t)
defer cleanupTmpIndexPath(t, tmpIndexPath)

config := DefaultConfig(tmpIndexPath)
indexWriter, err := OpenWriter(config)
if err != nil {
t.Fatal(err)
}
defer func() {
err = indexWriter.Close()
if err != nil {
t.Fatal(err)
}
}()

// create 1025 documents in a batch
// this should require more than one chunk in doc values
batch := NewBatch()
for i := 0; i < 1025; i++ {
doc := NewDocument(fmt.Sprintf("%d", i)).
AddField(NewTextField("name", "marty").Sortable())
batch.Update(doc.ID(), doc)
}

err = indexWriter.Batch(batch)
if err != nil {
t.Error(err)
}

reader, err := indexWriter.Reader()
if err != nil {
t.Fatal(err)
}
defer func() {
err = reader.Close()
if err != nil {
t.Fatal(err)
}
}()

q := NewTermQuery("marty").SetField("name")
req := NewTopNSearch(2000, q).SortBy([]string{"_id"})

dmi, err := reader.Search(context.Background(), req)
if err != nil {
t.Fatal(err)
}

next, err := dmi.Next()
for err == nil && next != nil {
var id string
err = next.VisitStoredFields(func(field string, value []byte) bool {
if field == "_id" {
id = string(value)
return false
}
return true
})
if err != nil {
t.Fatal(err)
}
if string(next.SortValue[0]) != id {
t.Fatalf("expected id '%s' to match sort value '%s'", id, string(next.SortValue[0]))
}
next, err = dmi.Next()
}
if err != nil {
t.Fatal(err)
}
}
5 changes: 4 additions & 1 deletion search/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ func (o SortOrder) Reverse() {

func (o SortOrder) Compute(match *DocumentMatch) {
for _, sort := range o {
match.SortValue = append(match.SortValue, sort.Value(match))
sortVal := sort.Value(match)
sortValCopy := make([]byte, len(sortVal))
copy(sortValCopy, sortVal)
match.SortValue = append(match.SortValue, sortValCopy)
}
}

Expand Down

0 comments on commit f34c4ac

Please sign in to comment.