internal/postgres: put Search args in a struct

There are enough arguments to Search that it is clearer to put them in
an options struct.

Also, add an option for DB query limit that is distinct from the
MaxResults option (but defaults to it, as currently.)

Change-Id: I191f2cb8d5e1eb49ec96af109ef1f1ee51fc3bdd
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/334251
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>

3 files changed, 28 insertions, 20 deletions

diff --git a/internal/postgres/benchmarks_test.go b/internal/postgres/benchmarks_test.go
index 16bd8e8b..ca4531cb 100644
--- a/internal/postgres/benchmarks_test.go
+++ b/internal/postgres/benchmarks_test.go
@@ -43,14 +43,14 @@ func BenchmarkSearch(b *testing.B) {
 		b.Fatal(err)
 	}
 	db := New(ddb)
-	searchers := map[string]func(context.Context, string, int, int, int, bool) ([]*internal.SearchResult, error){
+	searchers := map[string]func(context.Context, string, SearchOptions) ([]*internal.SearchResult, error){
 		"db.Search": db.Search,
 	}
 	for name, search := range searchers {
 		for _, query := range testQueries {
 			b.Run(name+":"+query, func(b *testing.B) {
 				for i := 0; i < b.N; i++ {
-					if _, err := search(ctx, query, 10, 0, 100, false); err != nil {
+					if _, err := search(ctx, query, SearchOptions{MaxResults: 10, MaxResultCount: 100}); err != nil {
 						b.Fatal(err)
 					}
 				}
diff --git a/internal/postgres/search.go b/internal/postgres/search.go
index c555de1f..c19b6b57 100644
--- a/internal/postgres/search.go
+++ b/internal/postgres/search.go
@@ -96,6 +96,19 @@ var symbolSearchers = map[string]searcher{
 	"symbol": (*DB).symbolSearch,
 }
 
+type SearchOptions struct {
+	// Maximum number of results to return (page size).
+	MaxResults int
+	// Limit for the DB query; defaults to MaxResults.
+	Limit int
+	// Offset for DB query.
+	Offset int
+	// Maximum number to use for total result count.
+	MaxResultCount int
+	// If true, perform a symbol search.
+	SearchSymbols bool
+}
+
 // Search executes two search requests concurrently:
 //   - a sequential scan of packages in descending order of popularity.
 //   - all packages ("deep" search) using an inverted index to filter to search
@@ -119,26 +132,21 @@ var symbolSearchers = map[string]searcher{
 // The gap in this optimization is search terms that are very frequent, but
 // rarely relevant: "int" or "package", for example. In these cases we'll pay
 // the penalty of a deep search that scans nearly every package.
-func (db *DB) Search(ctx context.Context, q string, maxResults, offset, maxResultCount int, searchSymbols bool) (_ []*internal.SearchResult, err error) {
-	defer derrors.WrapStack(&err, "DB.Search(ctx, %q, %d, %d)", q, maxResults, offset)
-
-	limit := maxResults
-	if experiment.IsActive(ctx, internal.ExperimentSearchGrouping) {
-		// Gather extra results for better grouping by module and series.
-		// Since deep search is using incremental querying, we can make this large.
-		// TODO(jba): For performance, modify the popular_search stored procedure.
-		limit *= 100
+func (db *DB) Search(ctx context.Context, q string, opts SearchOptions) (_ []*internal.SearchResult, err error) {
+	defer derrors.WrapStack(&err, "DB.Search(ctx, %q, %+v)", q, opts)
+	if opts.Limit == 0 {
+		opts.Limit = opts.MaxResults
 	}
 
 	var searchers map[string]searcher
-	if searchSymbols &&
+	if opts.SearchSymbols &&
 		experiment.IsActive(ctx, internal.ExperimentSearchGrouping) &&
 		experiment.IsActive(ctx, internal.ExperimentSymbolSearch) {
 		searchers = symbolSearchers
 	} else {
 		searchers = pkgSearchers
 	}
-	resp, err := db.hedgedSearch(ctx, q, limit, offset, maxResultCount, searchers, nil)
+	resp, err := db.hedgedSearch(ctx, q, opts.Limit, opts.Offset, opts.MaxResultCount, searchers, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -153,11 +161,11 @@ func (db *DB) Search(ctx context.Context, q string, maxResults, offset, maxResul
 			results = append(results, r)
 		}
 	}
-	if experiment.IsActive(ctx, internal.ExperimentSearchGrouping) && !searchSymbols {
+	if experiment.IsActive(ctx, internal.ExperimentSearchGrouping) && !opts.SearchSymbols {
 		results = groupSearchResults(results)
 	}
-	if len(results) > maxResults {
-		results = results[:maxResults]
+	if len(results) > opts.MaxResults {
+		results = results[:opts.MaxResults]
 	}
 	return results, nil
 }
@@ -300,7 +308,7 @@ func (db *DB) deepSearch(ctx context.Context, q string, limit, offset, maxResult
 			}
 			return nil
 		}
-		const fetchSize = 10 // number of rows to fetch at a time
+		const fetchSize = 20 // number of rows to fetch at a time
 		err = db.db.RunQueryIncrementally(ctx, query, fetchSize, collect, q, limit, offset)
 	} else {
 		collect := func(rows *sql.Rows) error {
diff --git a/internal/postgres/search_test.go b/internal/postgres/search_test.go
index bcf532e5..bd8e4b2a 100644
--- a/internal/postgres/search_test.go
+++ b/internal/postgres/search_test.go
@@ -661,7 +661,7 @@ func TestExcludedFromSearch(t *testing.T) {
 		t.Fatal(err)
 	}
 	// Search for both packages.
-	gotResults, err := testDB.Search(ctx, domain, 10, 0, 100, false)
+	gotResults, err := testDB.Search(ctx, domain, SearchOptions{MaxResults: 10})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -693,7 +693,7 @@ func TestSearchBypass(t *testing.T) {
 		{testDB, true},
 		{bypassDB, false},
 	} {
-		rs, err := test.db.Search(ctx, m.ModulePath, 10, 0, 100, false)
+		rs, err := test.db.Search(ctx, m.ModulePath, SearchOptions{MaxResults: 10})
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -719,7 +719,7 @@ func TestSearchLicenseDedup(t *testing.T) {
 		},
 	})
 	MustInsertModule(ctx, t, testDB, m)
-	got, err := testDB.Search(ctx, m.ModulePath, 10, 0, 100, false)
+	got, err := testDB.Search(ctx, m.ModulePath, SearchOptions{MaxResults: 10})
 	if err != nil {
 		t.Fatal(err)
 	}