code review

Author: CascadingRadium

document/document.go

@@ -31,7 +31,7 @@ func init() {
 }
 
 type Document struct {
-	id               string      `json:"id"`
+	id               string
 	Fields           []Field     `json:"fields"`
 	NestedDocuments  []*Document `json:"nested_documents"`
 	CompositeFields  []*CompositeField

index/scorch/snapshot_index.go

@@ -1238,8 +1238,8 @@ func (i *IndexSnapshot) Ancestors(ID index.IndexInternalID) ([]index.IndexIntern
 	rv[0] = index.NewIndexInternalID(nil, ldoc+globalOffset)
 
 	// then all ancestors shifted by +1
-	for j := 1; j < len(ancestors)+1; j++ {
-		rv[j] = index.NewIndexInternalID(nil, ancestors[j-1]+globalOffset)
+	for j := 0; j < len(ancestors); j++ {
+		rv[j+1] = index.NewIndexInternalID(nil, ancestors[j]+globalOffset)
 	}
 	return rv, nil
 }

mapping/document.go

@@ -455,9 +455,9 @@ func (dm *DocumentMapping) walkDocument(data interface{}, path []string, indexes
 					nestedContext := context.im.newWalkContext(nestedDocument, dm)
 					dm.processProperty(fieldVal, path, append(indexes, uint64(i)), nestedContext)
 					context.doc.AddNestedDocument(nestedDocument)
-					continue
+				} else {
+					dm.processProperty(fieldVal, path, append(indexes, uint64(i)), context)
 				}
-				dm.processProperty(fieldVal, path, append(indexes, uint64(i)), context)
 			}
 		}
 	case reflect.Ptr:

search/collector/eligible.go

@@ -144,7 +144,7 @@ func (ec *EligibleCollector) Collect(ctx context.Context, searcher search.Search
 	}
 
 	if ec.nestedStore != nil {
-		var count int
+		var count uint64
 		err := ec.nestedStore.VisitRoots(func(doc *search.DocumentMatch) error {
 			// process the root document
 			if err := dmHandler(doc); err != nil {
@@ -156,7 +156,7 @@ func (ec *EligibleCollector) Collect(ctx context.Context, searcher search.Search
 		if err != nil {
 			return err
 		}
-		ec.total = uint64(count)
+		ec.total = count
 	}
 
 	// help finalize/flush the results in case

search/collector/knn.go

@@ -223,7 +223,7 @@ func (hc *KNNCollector) Collect(ctx context.Context, searcher search.Searcher, r
 	}
 
 	if hc.nestedStore != nil {
-		var count int
+		var count uint64
 		err := hc.nestedStore.VisitRoots(func(doc *search.DocumentMatch) error {
 			// process the root document
 			if err := dmHandler(doc); err != nil {
@@ -235,7 +235,7 @@ func (hc *KNNCollector) Collect(ctx context.Context, searcher search.Searcher, r
 		if err != nil {
 			return err
 		}
-		hc.total = uint64(count)
+		hc.total = count
 	}
 
 	// help finalize/flush the results in case

search/collector/topn.go

@@ -90,15 +90,15 @@ const CheckDoneEvery = uint64(1024)
 // skipping over the first 'skip' hits
 // ordering hits by the provided sort order
 func NewTopNCollector(size int, skip int, sort search.SortOrder) *TopNCollector {
-	return newTopNCollector(size, skip, sort)
+	return newTopNCollector(size, skip, sort, nil)
 }
 
 // NewTopNCollectorAfter builds a collector to find the top 'size' hits
 // skipping over the first 'skip' hits
 // ordering hits by the provided sort order
 // starting after the provided 'after' sort values
 func NewTopNCollectorAfter(size int, sort search.SortOrder, after []string) *TopNCollector {
-	rv := newTopNCollector(size, 0, sort)
+	rv := newTopNCollector(size, 0, sort, nil)
 	rv.searchAfter = createSearchAfterDocument(sort, after)
 	return rv
 }
@@ -109,7 +109,7 @@ func NewTopNCollectorAfter(size int, sort search.SortOrder, after []string) *Top
 // while ensuring the nested documents are handled correctly
 // (i.e. parent document is returned instead of nested document)
 func NewNestedTopNCollector(size int, skip int, sort search.SortOrder, nr index.NestedReader) *TopNCollector {
-	return newNestedTopNCollector(size, skip, sort, nr)
+	return newTopNCollector(size, skip, sort, nr)
 }
 
 // NewNestedTopNCollectorAfter builds a collector to find the top 'size' hits
@@ -119,37 +119,21 @@ func NewNestedTopNCollector(size int, skip int, sort search.SortOrder, nr index.
 // while ensuring the nested documents are handled correctly
 // (i.e. parent document is returned instead of nested document)
 func NewNestedTopNCollectorAfter(size int, sort search.SortOrder, after []string, nr index.NestedReader) *TopNCollector {
-	rv := newNestedTopNCollector(size, 0, sort, nr)
+	rv := newTopNCollector(size, 0, sort, nr)
 	rv.searchAfter = createSearchAfterDocument(sort, after)
 	return rv
 }
 
-func newTopNCollector(size int, skip int, sort search.SortOrder) *TopNCollector {
+func newTopNCollector(size int, skip int, sort search.SortOrder, nr index.NestedReader) *TopNCollector {
 	hc := &TopNCollector{size: size, skip: skip, sort: sort}
 
 	hc.store = getOptimalCollectorStore(size, skip, func(i, j *search.DocumentMatch) int {
 		return hc.sort.Compare(hc.cachedScoring, hc.cachedDesc, i, j)
 	})
 
-	// these lookups traverse an interface, so do once up-front
-	if sort.RequiresDocID() {
-		hc.needDocIds = true
+	if nr != nil {
+		hc.nestedStore = newStoreNested(nr)
 	}
-	hc.neededFields = sort.RequiredFields()
-	hc.cachedScoring = sort.CacheIsScore()
-	hc.cachedDesc = sort.CacheDescending()
-
-	return hc
-}
-
-func newNestedTopNCollector(size int, skip int, sort search.SortOrder, nr index.NestedReader) *TopNCollector {
-	hc := &TopNCollector{size: size, skip: skip, sort: sort}
-
-	hc.store = getOptimalCollectorStore(size, skip, func(i, j *search.DocumentMatch) int {
-		return hc.sort.Compare(hc.cachedScoring, hc.cachedDesc, i, j)
-	})
-
-	hc.nestedStore = newStoreNested(nr)
 
 	// these lookups traverse an interface, so do once up-front
 	if sort.RequiresDocID() {
@@ -369,7 +353,7 @@ func (hc *TopNCollector) Collect(ctx context.Context, searcher search.Searcher,
 	}
 
 	if hc.nestedStore != nil {
-		var count int
+		var count uint64
 		err := hc.nestedStore.VisitRoots(func(doc *search.DocumentMatch) error {
 			if err := hc.adjustDocumentMatch(searchContext, reader, doc); err != nil {
 				return err
@@ -386,7 +370,7 @@ func (hc *TopNCollector) Collect(ctx context.Context, searcher search.Searcher,
 		if err != nil {
 			return err
 		}
-		hc.total = uint64(count)
+		hc.total = count
 	}
 
 	if hc.knnHits != nil {

search/explanation.go

@@ -57,6 +57,12 @@ func (expl *Explanation) Size() int {
 
 const MergedExplMessage = "sum of merged explanations:"
 
+// MergeExpl merges two explanations into one.
+// If either explanation is nil, the other is returned.
+// If the first explanation is already a merged explanation,
+// the second explanation is appended to its children.
+// Otherwise, a new merged explanation is created
+// with the two explanations as its children.
 func MergeExpl(first, second *Explanation) *Explanation {
 	if first == nil {
 		return second
@@ -70,12 +76,6 @@ func MergeExpl(first, second *Explanation) *Explanation {
 		first.Children = append(first.Children, second)
 		return first
 	}
-	if second.Message == MergedExplMessage {
-		// reuse second explanation as the merged one
-		second.Value += first.Value
-		second.Children = append(second.Children, first)
-		return second
-	}
 	// create a new explanation to hold the merged one
 	rv := &Explanation{
 		Value:    first.Value + second.Value,