From 16b568a81b8a773b56061a899b12d6792426b251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Tue, 18 Mar 2025 19:40:01 +0100 Subject: [PATCH 1/7] fix: use copy(to, from) instead of a loop search/searcher/search_conjunction.go:53:2: S1001: should use copy(to, from) instead of a loop (gosimple) search/searcher/search_disjunction_slice.go:82:3: S1001: should use copy(to, from) instead of a loop (gosimple) --- search/searcher/search_conjunction.go | 7 +++---- search/searcher/search_disjunction_slice.go | 16 +++++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/search/searcher/search_conjunction.go b/search/searcher/search_conjunction.go index 25e661075..57d8855ee 100644 --- a/search/searcher/search_conjunction.go +++ b/search/searcher/search_conjunction.go @@ -47,12 +47,11 @@ type ConjunctionSearcher struct { func NewConjunctionSearcher(ctx context.Context, indexReader index.IndexReader, qsearchers []search.Searcher, options search.SearcherOptions) ( - search.Searcher, error) { + search.Searcher, error, +) { // build the sorted downstream searchers searchers := make(OrderedSearcherList, len(qsearchers)) - for i, searcher := range qsearchers { - searchers[i] = searcher - } + copy(searchers, qsearchers) sort.Sort(searchers) // attempt the "unadorned" conjunction optimization only when we diff --git a/search/searcher/search_disjunction_slice.go b/search/searcher/search_disjunction_slice.go index fadfa59ca..6a92ffa09 100644 --- a/search/searcher/search_disjunction_slice.go +++ b/search/searcher/search_disjunction_slice.go @@ -52,7 +52,8 @@ type DisjunctionSliceSearcher struct { func newDisjunctionSliceSearcher(ctx context.Context, indexReader index.IndexReader, qsearchers []search.Searcher, min float64, options search.SearcherOptions, limit bool) ( - *DisjunctionSliceSearcher, error) { + *DisjunctionSliceSearcher, error, +) { if limit && tooManyClauses(len(qsearchers)) { return nil, tooManyClausesErr("", len(qsearchers)) } @@ -79,9 +80,7 @@ func newDisjunctionSliceSearcher(ctx context.Context, indexReader index.IndexRea originalPos = sortedSearchers.index } else { searchers = make(OrderedSearcherList, len(qsearchers)) - for i, searcher := range qsearchers { - searchers[i] = searcher - } + copy(searchers, qsearchers) sort.Sort(searchers) } @@ -210,7 +209,8 @@ func (s *DisjunctionSliceSearcher) SetQueryNorm(qnorm float64) { } func (s *DisjunctionSliceSearcher) Next(ctx *search.SearchContext) ( - *search.DocumentMatch, error) { + *search.DocumentMatch, error, +) { if !s.initialized { err := s.initSearchers(ctx) if err != nil { @@ -255,7 +255,8 @@ func (s *DisjunctionSliceSearcher) Next(ctx *search.SearchContext) ( } func (s *DisjunctionSliceSearcher) Advance(ctx *search.SearchContext, - ID index.IndexInternalID) (*search.DocumentMatch, error) { + ID index.IndexInternalID, +) (*search.DocumentMatch, error) { if !s.initialized { err := s.initSearchers(ctx) if err != nil { @@ -320,7 +321,8 @@ func (s *DisjunctionSliceSearcher) DocumentMatchPoolSize() int { // but only activates on an edge case where the disjunction is a // wrapper around a single Optimizable child searcher func (s *DisjunctionSliceSearcher) Optimize(kind string, octx index.OptimizableContext) ( - index.OptimizableContext, error) { + index.OptimizableContext, error, +) { if len(s.searchers) == 1 { o, ok := s.searchers[0].(index.Optimizable) if ok { From edeca95b1868db136e2e1e60a008ebeeb88950f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Tue, 18 Mar 2025 20:08:44 +0100 Subject: [PATCH 2/7] fix: ineffectual assignment to expectedCount index/scorch/reader_test.go:72:2: ineffectual assignment to expectedCount (ineffassign) --- index/scorch/reader_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/index/scorch/reader_test.go b/index/scorch/reader_test.go index 18efefc38..a52333469 100644 --- a/index/scorch/reader_test.go +++ b/index/scorch/reader_test.go @@ -101,7 +101,6 @@ func TestIndexReader(t *testing.T) { t.Errorf("Error accessing term field reader: %v", err) } - expectedCount = 2 count = reader.Count() if count != expectedCount { t.Errorf("Expected doc count to be: %d got: %d", expectedCount, count) From 4bc8f7829ba2c408b8c12ee7c439a88c2bf94699 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Tue, 18 Mar 2025 20:10:49 +0100 Subject: [PATCH 3/7] fix: ineffectual assignment to ok search/sort.go:49:14: ineffectual assignment to ok (ineffassign) test: add test tables for function ParseSearchSortObj test: add unit test before the vadilation of the ok variable and test it after the change --- search/sort.go | 14 +- search/sort_test.go | 338 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 349 insertions(+), 3 deletions(-) create mode 100644 search/sort_test.go diff --git a/search/sort.go b/search/sort.go index b13fa16c1..2b757c48d 100644 --- a/search/sort.go +++ b/search/sort.go @@ -28,8 +28,10 @@ import ( "github.com/blevesearch/bleve/v2/util" ) -var HighTerm = strings.Repeat(string(utf8.MaxRune), 3) -var LowTerm = string([]byte{0x00}) +var ( + HighTerm = strings.Repeat(string(utf8.MaxRune), 3) + LowTerm = string([]byte{0x00}) +) type SearchSort interface { UpdateVisitor(field string, term []byte) @@ -47,10 +49,15 @@ type SearchSort interface { func ParseSearchSortObj(input map[string]interface{}) (SearchSort, error) { descending, ok := input["desc"].(bool) + if !ok { + descending = false + } + by, ok := input["by"].(string) if !ok { return nil, fmt.Errorf("search sort must specify by") } + switch by { case "id": return &SortDocID{ @@ -612,7 +619,8 @@ var maxDistance = string(numeric.MustNewPrefixCodedInt64(math.MaxInt64, 0)) // NewSortGeoDistance creates SearchSort instance for sorting documents by // their distance from the specified point. func NewSortGeoDistance(field, unit string, lon, lat float64, desc bool) ( - *SortGeoDistance, error) { + *SortGeoDistance, error, +) { rv := &SortGeoDistance{ Field: field, Desc: desc, diff --git a/search/sort_test.go b/search/sort_test.go new file mode 100644 index 000000000..009444b5d --- /dev/null +++ b/search/sort_test.go @@ -0,0 +1,338 @@ +package search + +import ( + "reflect" + "testing" +) + +func TestParseSearchSortObj(t *testing.T) { + tests := []struct { + name string + input map[string]interface{} + want SearchSort + wantErr bool + }{ + { + name: "sort by id", + input: map[string]interface{}{ + "by": "id", + "desc": false, + }, + want: &SortDocID{ + Desc: false, + }, + wantErr: false, + }, + { + name: "sort by id descending", + input: map[string]interface{}{ + "by": "id", + "desc": true, + }, + want: &SortDocID{ + Desc: true, + }, + wantErr: false, + }, + { + name: "sort by score", + input: map[string]interface{}{ + "by": "score", + "desc": false, + }, + want: &SortScore{ + Desc: false, + }, + wantErr: false, + }, + { + name: "sort by score descending", + input: map[string]interface{}{ + "by": "score", + "desc": true, + }, + want: &SortScore{ + Desc: true, + }, + wantErr: false, + }, + { + name: "sort by geo_distance", + input: map[string]interface{}{ + "by": "geo_distance", + "field": "location", + "location": map[string]interface{}{ + "lon": 1.0, + "lat": 2.0, + }, + "unit": "km", + "desc": false, + }, + want: &SortGeoDistance{ + Field: "location", + Desc: false, + Lon: 1.0, + Lat: 2.0, + Unit: "km", + unitMult: 1000.0, + }, + wantErr: false, + }, + { + name: "sort by field", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "desc": false, + "type": "auto", + "mode": "default", + "missing": "last", + }, + want: &SortField{ + Field: "name", + Desc: false, + Type: SortFieldAuto, + Mode: SortFieldDefault, + Missing: SortFieldMissingLast, + }, + wantErr: false, + }, + { + name: "sort by field with missing", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "desc": false, + "type": "auto", + "mode": "default", + "missing": "first", + }, + want: &SortField{ + Field: "name", + Desc: false, + Type: SortFieldAuto, + Mode: SortFieldDefault, + Missing: SortFieldMissingFirst, + }, + wantErr: false, + }, + { + name: "sort by field descending", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "desc": true, + "type": "string", + "mode": "min", + "missing": "first", + }, + want: &SortField{ + Field: "name", + Desc: true, + Type: SortFieldAsString, + Mode: SortFieldMin, + Missing: SortFieldMissingFirst, + }, + wantErr: false, + }, + { + name: "missing by", + input: map[string]interface{}{ + "desc": true, + }, + want: nil, + wantErr: true, + }, + { + name: "unknown by", + input: map[string]interface{}{ + "by": "unknown", + }, + want: nil, + wantErr: true, + }, + { + name: "missing field for geo_distance", + input: map[string]interface{}{ + "by": "geo_distance", + "location": map[string]interface{}{ + "lon": 1.0, + "lat": 2.0, + }, + }, + want: nil, + wantErr: true, + }, + { + name: "missing location for geo_distance", + input: map[string]interface{}{ + "by": "geo_distance", + "field": "location", + }, + want: nil, + wantErr: true, + }, + { + name: "invalid unit for geo_distance", + input: map[string]interface{}{ + "by": "geo_distance", + "field": "location", + "location": map[string]interface{}{ + "lon": 1.0, + "lat": 2.0, + }, + "unit": "invalid", + }, + want: nil, + wantErr: true, + }, + { + name: "missing field for field sort", + input: map[string]interface{}{ + "by": "field", + }, + want: nil, + wantErr: true, + }, + { + name: "unknown type for field sort", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "type": "unknown", + }, + want: nil, + wantErr: true, + }, + { + name: "number type for field sort with desc", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "type": "number", + "mode": "default", + "desc": true, + "missing": "last", + }, + want: &SortField{ + Field: "name", + Desc: true, + Type: SortFieldAsNumber, + Mode: SortFieldDefault, + Missing: SortFieldMissingLast, + }, + wantErr: false, + }, + { + name: "date type for field sort with desc", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "type": "date", + "mode": "default", + "desc": true, + "missing": "last", + }, + want: &SortField{ + Field: "name", + Desc: true, + Type: SortFieldAsDate, + Mode: SortFieldDefault, + Missing: SortFieldMissingLast, + }, + wantErr: false, + }, + { + name: "unknown type for field sort with missing", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "type": "unknown", + "mode": "default", + "missing": "last", + }, + want: nil, + wantErr: true, + }, + { + name: "unknown mode for field sort", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "mode": "unknown", + }, + want: nil, + wantErr: true, + }, + { + name: "default mode for field sort", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "mode": "default", + }, + want: &SortField{ + Field: "name", + Desc: false, + Type: SortFieldAuto, + Mode: SortFieldDefault, + Missing: SortFieldMissingLast, + }, + wantErr: false, + }, + { + name: "max mode for field sort", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "mode": "max", + }, + want: &SortField{ + Field: "name", + Desc: false, + Type: SortFieldAuto, + Mode: SortFieldMax, + Missing: SortFieldMissingLast, + }, + wantErr: false, + }, + { + name: "min mode for field sort", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "mode": "min", + }, + want: &SortField{ + Field: "name", + Desc: false, + Type: SortFieldAuto, + Mode: SortFieldMin, + Missing: SortFieldMissingLast, + }, + wantErr: false, + }, + { + name: "unknown missing for field sort", + input: map[string]interface{}{ + "by": "field", + "field": "name", + "missing": "unknown", + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseSearchSortObj(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("ParseSearchSortObj() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ParseSearchSortObj() = %v, want %v", got, tt.want) + } + }) + } +} From 652bc8225d6805e0601555000bd410df41a406a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Tue, 18 Mar 2025 20:14:03 +0100 Subject: [PATCH 4/7] fix: ineffectual assignment to arrayPositionsLen index/upsidedown/row.go:645:7: ineffectual assignment to arrayPositionsLen (ineffassign) --- index/upsidedown/row.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/index/upsidedown/row.go b/index/upsidedown/row.go index fff6d0673..a4dccc841 100644 --- a/index/upsidedown/row.go +++ b/index/upsidedown/row.go @@ -26,8 +26,10 @@ import ( "github.com/golang/protobuf/proto" ) -var reflectStaticSizeTermFrequencyRow int -var reflectStaticSizeTermVector int +var ( + reflectStaticSizeTermFrequencyRow int + reflectStaticSizeTermVector int +) func init() { var tfr TermFrequencyRow @@ -322,7 +324,6 @@ func NewDictionaryRowKV(key, value []byte) (*DictionaryRow, error) { return nil, err } return rv, nil - } func NewDictionaryRowK(key []byte) (*DictionaryRow, error) { @@ -642,8 +643,7 @@ func (tfr *TermFrequencyRow) parseV(value []byte, includeTermVectors bool) error } currOffset += bytesRead - var arrayPositionsLen uint64 = 0 - arrayPositionsLen, bytesRead = binary.Uvarint(value[currOffset:]) + arrayPositionsLen, bytesRead := binary.Uvarint(value[currOffset:]) if bytesRead <= 0 { return fmt.Errorf("invalid term frequency value, vector contains no arrayPositionLen") } @@ -682,7 +682,6 @@ func NewTermFrequencyRowKV(key, value []byte) (*TermFrequencyRow, error) { return nil, err } return rv, nil - } type BackIndexRow struct { @@ -1029,7 +1028,7 @@ func visitBackIndexRow(data []byte, callback backIndexFieldTermVisitor) error { return io.ErrUnexpectedEOF } // don't track unrecognized data - //m.XXX_unrecognized = append(m.XXX_unrecognized, data[iNdEx:iNdEx+skippy]...) + // m.XXX_unrecognized = append(m.XXX_unrecognized, data[iNdEx:iNdEx+skippy]...) iNdEx += skippy } } @@ -1109,7 +1108,7 @@ func visitBackIndexRowFieldTerms(data []byte, callback backIndexFieldTermVisitor if postIndex > l { return io.ErrUnexpectedEOF } - //m.Terms = append(m.Terms, string(data[iNdEx:postIndex])) + // m.Terms = append(m.Terms, string(data[iNdEx:postIndex])) callback(theField, data[iNdEx:postIndex]) iNdEx = postIndex default: @@ -1132,7 +1131,7 @@ func visitBackIndexRowFieldTerms(data []byte, callback backIndexFieldTermVisitor if (iNdEx + skippy) > l { return io.ErrUnexpectedEOF } - //m.XXX_unrecognized = append(m.XXX_unrecognized, data[iNdEx:iNdEx+skippy]...) + // m.XXX_unrecognized = append(m.XXX_unrecognized, data[iNdEx:iNdEx+skippy]...) iNdEx += skippy } } From 863a9cbf20d3d5f2bc0596b7f1268edc89b7143f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Tue, 18 Mar 2025 20:30:04 +0100 Subject: [PATCH 5/7] fix: use zero value instead of assignment to zero --- index/upsidedown/row.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/index/upsidedown/row.go b/index/upsidedown/row.go index a4dccc841..622db46c1 100644 --- a/index/upsidedown/row.go +++ b/index/upsidedown/row.go @@ -643,7 +643,8 @@ func (tfr *TermFrequencyRow) parseV(value []byte, includeTermVectors bool) error } currOffset += bytesRead - arrayPositionsLen, bytesRead := binary.Uvarint(value[currOffset:]) + var arrayPositionsLen uint64 + arrayPositionsLen, bytesRead = binary.Uvarint(value[currOffset:]) if bytesRead <= 0 { return fmt.Errorf("invalid term frequency value, vector contains no arrayPositionLen") } From 9703f13fc45f621ab30b00050fd1d44b73389dea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Tue, 18 Mar 2025 20:33:29 +0100 Subject: [PATCH 6/7] fix: efficient assigment analysis/token/keyword/keyword.go:38:11: SA6001: m[string(key)] would be more efficient than k := string(key); m[k] (staticcheck) --- analysis/token/keyword/keyword.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/analysis/token/keyword/keyword.go b/analysis/token/keyword/keyword.go index 0f8c84775..f2e56f396 100644 --- a/analysis/token/keyword/keyword.go +++ b/analysis/token/keyword/keyword.go @@ -35,8 +35,7 @@ func NewKeyWordMarkerFilter(keyWords analysis.TokenMap) *KeyWordMarkerFilter { func (f *KeyWordMarkerFilter) Filter(input analysis.TokenStream) analysis.TokenStream { for _, token := range input { - word := string(token.Term) - _, isKeyWord := f.keyWords[word] + _, isKeyWord := f.keyWords[string(token.Term)] if isKeyWord { token.KeyWord = true } From ade02eda38d5639a39d709681a5ed57aee256824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Tue, 18 Mar 2025 21:07:32 +0100 Subject: [PATCH 7/7] fix: replace if statement with unconditional function strings.TrimPrefix search/query/regexp.go:72:2: S1017: should replace this 'if' statement with an unconditional 'strings.TrimPrefix' (gosimple) --- search/query/regexp.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/search/query/regexp.go b/search/query/regexp.go index 6b3da9554..189fd5f34 100644 --- a/search/query/regexp.go +++ b/search/query/regexp.go @@ -69,12 +69,9 @@ func (q *RegexpQuery) Searcher(ctx context.Context, i index.IndexReader, m mappi // known to interfere with LiteralPrefix() the way ^ does // and removing $ introduces possible ambiguities with escaped \$, \\$, etc actualRegexp := q.Regexp - if strings.HasPrefix(actualRegexp, "^") { - actualRegexp = actualRegexp[1:] // remove leading ^ - } + actualRegexp = strings.TrimPrefix(actualRegexp, "^") // remove leading ^ if it exists - return searcher.NewRegexpStringSearcher(ctx, i, actualRegexp, field, - q.BoostVal.Value(), options) + return searcher.NewRegexpStringSearcher(ctx, i, actualRegexp, field, q.BoostVal.Value(), options) } func (q *RegexpQuery) Validate() error {