Browse Source

Merge pull request #871 from steveyen/search-fix-findPhrasePaths

search_phrase findPhrasePaths() fix on slice reuse
Steve Yen 1 year ago
parent
commit
ebb6f91d41
2 changed files with 47 additions and 3 deletions
  1. 2 2
      search/searcher/search_phrase.go
  2. 45 1
      search/searcher/search_phrase_test.go

+ 2 - 2
search/searcher/search_phrase.go

@@ -288,7 +288,7 @@ func findPhrasePaths(prevPos uint64, ap search.ArrayPositions, phraseTerms [][]s
 
 	// no more terms
 	if len(phraseTerms) < 1 {
-		return []phrasePath{p}
+		return []phrasePath{append(phrasePath(nil), p...)}
 	}
 
 	car := phraseTerms[0]
@@ -320,7 +320,7 @@ func findPhrasePaths(prevPos uint64, ap search.ArrayPositions, phraseTerms [][]s
 				dist = editDistance(prevPos+1, loc.Pos)
 			}
 
-			// if enough slop reamining, continue recursively
+			// if enough slop remaining, continue recursively
 			if prevPos == 0 || (remainingSlop-dist) >= 0 {
 				// this location works, add it to the path (but not for empty term)
 				px := append(p, phrasePart{term: carTerm, loc: loc})

+ 45 - 1
search/searcher/search_phrase_test.go

@@ -360,6 +360,7 @@ func TestFindPhrasePathsSloppy(t *testing.T) {
 		phrase [][]string
 		paths  []phrasePath
 		slop   int
+		tlm    search.TermLocationMap
 	}{
 		// no match
 		{
@@ -454,10 +455,53 @@ func TestFindPhrasePathsSloppy(t *testing.T) {
 				},
 			},
 		},
+		// test an append() related edge case, where append()'s
+		// current behavior needs to be called 3 times starting from a
+		// nil slice before it grows to a slice with extra capacity --
+		// hence, 3 initial terms of ark, bat, cat
+		{
+			phrase: [][]string{
+				[]string{"ark"}, []string{"bat"}, []string{"cat"}, []string{"dog"},
+			},
+			slop: 1,
+			paths: []phrasePath{
+				phrasePath{
+					phrasePart{"ark", &search.Location{Pos: 1}},
+					phrasePart{"bat", &search.Location{Pos: 2}},
+					phrasePart{"cat", &search.Location{Pos: 3}},
+					phrasePart{"dog", &search.Location{Pos: 4}},
+				},
+				phrasePath{
+					phrasePart{"ark", &search.Location{Pos: 1}},
+					phrasePart{"bat", &search.Location{Pos: 2}},
+					phrasePart{"cat", &search.Location{Pos: 3}},
+					phrasePart{"dog", &search.Location{Pos: 5}},
+				},
+			},
+			tlm: search.TermLocationMap{ // ark bat cat dog dog
+				"ark": search.Locations{
+					&search.Location{Pos: 1},
+				},
+				"bat": search.Locations{
+					&search.Location{Pos: 2},
+				},
+				"cat": search.Locations{
+					&search.Location{Pos: 3},
+				},
+				"dog": search.Locations{
+					&search.Location{Pos: 4},
+					&search.Location{Pos: 5},
+				},
+			},
+		},
 	}
 
 	for i, test := range tests {
-		actualPaths := findPhrasePaths(0, nil, test.phrase, tlm, nil, test.slop)
+		tlmToUse := test.tlm
+		if tlmToUse == nil {
+			tlmToUse = tlm
+		}
+		actualPaths := findPhrasePaths(0, nil, test.phrase, tlmToUse, nil, test.slop)
 		if !reflect.DeepEqual(actualPaths, test.paths) {
 			t.Fatalf("expected: %v got %v for test %d", test.paths, actualPaths, i)
 		}