Skip to content

Commit e5e6f65

Browse files
jbagopherbot
authored andcommitted
internal/postgres: paginate versions query
Add start and limit args to the versions query so it can be paginated. Change-Id: I2f96911f74f91da4726127f87fff75f6fc7141f1 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/780340 Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> Auto-Submit: Jonathan Amsterdam <jba@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> kokoro-CI: kokoro <noreply+kokoro@google.com>
1 parent d92682e commit e5e6f65

4 files changed

Lines changed: 181 additions & 50 deletions

File tree

internal/postgres/delete_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,14 +178,14 @@ func TestDeletePseudoversionsExcept(t *testing.T) {
178178
if err := testDB.DeletePseudoversionsExcept(ctx, sample.ModulePath, pseudo1); err != nil {
179179
t.Fatal(err)
180180
}
181-
mods, err := getPathVersions(ctx, testDB, sample.ModulePath, version.TypeRelease)
181+
mods, _, err := getPathVersions(ctx, testDB, sample.ModulePath, "", 800, version.TypeRelease)
182182
if err != nil {
183183
t.Fatal(err)
184184
}
185185
if len(mods) != 1 && mods[0].Version != sample.VersionString {
186186
t.Errorf("module version %q was not found", sample.VersionString)
187187
}
188-
mods, err = getPathVersions(ctx, testDB, sample.ModulePath, version.TypePseudo)
188+
mods, _, err = getPathVersions(ctx, testDB, sample.ModulePath, "", 10, version.TypePseudo)
189189
if err != nil {
190190
t.Fatal(err)
191191
}

internal/postgres/details.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,15 @@ func (s jsonbScanner) Scan(value any) (err error) {
188188
return nil
189189
}
190190

191-
// scanModuleInfo constructs an *internal.ModuleInfo from the given scanner.
192-
func scanModuleInfo(scan func(dest ...any) error) (*internal.ModuleInfo, error) {
191+
// scanModuleInfo constructs an *internal.ModuleInfo from the
192+
// given scanner. The extras argument holds the destinations for
193+
// additional columns.
194+
func scanModuleInfo(scan func(dest ...any) error, extras ...any) (*internal.ModuleInfo, error) {
193195
var mi internal.ModuleInfo
194-
if err := scan(&mi.ModulePath, &mi.Version, &mi.CommitTime,
195-
&mi.IsRedistributable, &mi.HasGoMod, jsonbScanner{&mi.SourceInfo}); err != nil {
196+
args := []any{&mi.ModulePath, &mi.Version, &mi.CommitTime,
197+
&mi.IsRedistributable, &mi.HasGoMod, jsonbScanner{&mi.SourceInfo}}
198+
args = append(args, extras...)
199+
if err := scan(args...); err != nil {
196200
return nil, err
197201
}
198202
return &mi, nil

internal/postgres/version.go

Lines changed: 73 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"database/sql"
1010
"errors"
1111
"fmt"
12+
"io"
13+
"strconv"
1214
"strings"
1315

1416
"github.com/Masterminds/squirrel"
@@ -30,25 +32,43 @@ func (db *DB) GetVersionsForPath(ctx context.Context, path string) (_ []*interna
3032
defer derrors.WrapStack(&err, "GetVersionsForPath(ctx, %q)", path)
3133
defer stats.Elapsed(ctx, "GetVersionsForPath")()
3234

33-
versions, err := getPathVersions(ctx, db, path, version.TypeRelease, version.TypePrerelease)
35+
// When a page shows too many versions, it can result in a Chrome CSS
36+
// bug: https://bugs.chromium.org/p/chromium/issues/detail?id=688640.
37+
// For example,
38+
// https://pkg.go.dev/github.com/aws/aws-sdk-go/aws/signer/v4?tab=versions.
39+
// It's not that useful to see that many versions on a page anyway, so
40+
// just limit to 800 versions.
41+
versions, lsv, err := getPathVersions(ctx, db, path, "", 800, version.TypeRelease, version.TypePrerelease)
3442
if err != nil {
3543
return nil, err
3644
}
3745
if len(versions) != 0 {
3846
return versions, nil
3947
}
40-
versions, err = getPathVersions(ctx, db, path, version.TypePseudo)
48+
versions, _, err = getPathVersions(ctx, db, path, "", 10, version.TypePseudo)
4149
if err != nil {
4250
return nil, err
4351
}
52+
// To satisfy unparam until a subsequent CL.
53+
// TODO(jba); remove this.
54+
fmt.Fprint(io.Discard, lsv)
4455
return versions, nil
4556
}
4657

4758
// getPathVersions returns a list of versions sorted in descending semver
4859
// order. The version types included in the list are specified by a list of
4960
// VersionTypes.
50-
func getPathVersions(ctx context.Context, db *DB, path string, versionTypes ...version.Type) (_ []*internal.ModuleInfo, err error) {
51-
defer derrors.WrapStack(&err, "getPathVersions(ctx, db, %q, %v)", path, versionTypes)
61+
// The result can be paginated by passing pageToken, which should be either the empty
62+
// string or a value returned from a previous call.
63+
// Each subsequent page will begin with the last value of the previous page.
64+
func getPathVersions(ctx context.Context, db *DB, path string, startPageToken string, limit int, versionTypes ...version.Type) (_ []*internal.ModuleInfo, nextPageToken string, err error) {
65+
defer derrors.WrapStack(&err, "getPathVersions(ctx, db, %q, %q, %d, %v)", path, startPageToken, limit, versionTypes)
66+
67+
// Get previous values from pageToken.
68+
var pageTokenArgs = []any{false, "", ""}
69+
if startPageToken != "" {
70+
pageTokenArgs, err = parsePageToken(startPageToken)
71+
}
5272

5373
baseQuery := `
5474
SELECT
@@ -57,7 +77,10 @@ func getPathVersions(ctx context.Context, db *DB, path string, versionTypes ...v
5777
m.commit_time,
5878
m.redistributable,
5979
m.has_go_mod,
60-
m.source_info
80+
m.source_info,
81+
-- to construct the page token
82+
m.incompatible,
83+
m.sort_version
6184
FROM modules m
6285
INNER JOIN units u
6386
ON u.module_id = m.id
@@ -71,42 +94,69 @@ func getPathVersions(ctx context.Context, db *DB, path string, versionTypes ...v
7194
LIMIT 1
7295
)
7396
AND version_type in (%s)
97+
AND ($3 = '' OR (NOT m.incompatible, m.module_path, m.sort_version) <= (NOT $2, $3, $4))
7498
ORDER BY
7599
m.incompatible,
76100
m.module_path DESC,
77101
m.sort_version DESC %s`
78102

79-
queryEnd := `;`
80103
if len(versionTypes) == 0 {
81-
return nil, fmt.Errorf("error: must specify at least one version type")
82-
} else if len(versionTypes) == 1 && versionTypes[0] == version.TypePseudo {
83-
queryEnd = `LIMIT 10;`
84-
} else {
85-
// When a page shows too many versions, it can result in a Chrome CSS
86-
// bug: https://bugs.chromium.org/p/chromium/issues/detail?id=688640.
87-
// For example,
88-
// https://pkg.go.dev/github.com/aws/aws-sdk-go/aws/signer/v4?tab=versions.
89-
// It's not that useful to see that many versions on a page anyway, so
90-
// just limit to 800 versions.
91-
queryEnd = `LIMIT 800;`
104+
return nil, "", fmt.Errorf("error: must specify at least one version type")
105+
}
106+
queryEnd := ";"
107+
if limit > 0 {
108+
queryEnd = fmt.Sprintf("LIMIT %d;", limit)
92109
}
93110
query := fmt.Sprintf(baseQuery, versionTypeExpr(versionTypes), queryEnd)
94-
var versions []*internal.ModuleInfo
111+
112+
var (
113+
versions []*internal.ModuleInfo
114+
lastIncompatible bool
115+
lastSortVersion string
116+
)
95117
collect := func(rows *sql.Rows) error {
96-
mi, err := scanModuleInfo(rows.Scan)
118+
mi, err := scanModuleInfo(rows.Scan, &lastIncompatible, &lastSortVersion)
97119
if err != nil {
98120
return fmt.Errorf("row.Scan(): %v", err)
99121
}
100122
versions = append(versions, mi)
101123
return nil
102124
}
103-
if err := db.db.RunQuery(ctx, query, collect, path); err != nil {
104-
return nil, err
125+
args := append([]any{path}, pageTokenArgs...)
126+
if err := db.db.RunQuery(ctx, query, collect, args...); err != nil {
127+
return nil, "", err
105128
}
106129
if err := populateLatestInfos(ctx, db, versions); err != nil {
107-
return nil, err
130+
return nil, "", err
108131
}
109-
return versions, nil
132+
// Construct the page token for the next page.
133+
// See the comment near the top of this function for the format.
134+
if len(versions) > 0 {
135+
nextPageToken = makePageToken(lastIncompatible, versions[len(versions)-1].ModulePath, lastSortVersion)
136+
}
137+
return versions, nextPageToken, nil
138+
}
139+
140+
// parsePageToken parses a page token for getPathVersions.
141+
// It return a slice of query args.
142+
func parsePageToken(s string) (queryArgs []any, err error) {
143+
// A page token has the form "I P S"
144+
// where I is a bool for incompatible version, P is a module path, and S
145+
// is a sort version. Spaces suffice to separate these since none can contain a space.
146+
parts := strings.Fields(s)
147+
if len(parts) != 3 {
148+
return nil, errors.New("invalid page token (wrong # parts)")
149+
}
150+
startIncompatible, err := strconv.ParseBool(parts[0])
151+
if err != nil {
152+
return nil, fmt.Errorf("invalid page token: %v", err)
153+
}
154+
return []any{startIncompatible, parts[1], parts[2]}, nil
155+
}
156+
157+
// makePageToken constructs a page token for getPathVersions.
158+
func makePageToken(inc bool, mpath, version string) string {
159+
return fmt.Sprintf("%t %s %s", inc, mpath, version)
110160
}
111161

112162
// versionTypeExpr returns a comma-separated list of version types,

0 commit comments

Comments
 (0)