Pager.Take: take n rows without paying for a page it will not read

[ExaltedTrou6]

What and why

A live run of the SDK against a portal (50 deals in one batch + 45 of them listed through Pages) exposed that the natural way to write "take N rows" silently spends an extra HTTP request:

for p.Next(ctx) {                       // the 46th row is on the first page,
	for _, row := range p.Rows() {      // but the outer loop's condition is a
		if len(taken) == want { break } // call to the portal, and after break
		taken = append(taken, row)      // it is evaluated again
	}
}

break leaves the inner loop only. The outer condition is evaluated once more — and that is a request for a page nobody will read. The code compiles, it looks right, it hands over the right rows; the only thing wasted is a rate-limit token. The correct form is the bound in the loop header (for len(taken) < want && p.Next(ctx)), but one has to know it. This is exactly the class of mistake Pages was built for.

Pager.Take(ctx, n) is that form, packaged.

• It returns fewer than n rows only at the end of the list or on error; the error is returned alongside the rows it did read (a walk that failed on page three still read pages one and two).
• Rows fetched but not handed over are not thrown away: the next Take/Next hands them over without going to the portal. Take and Next mix freely.
• The cursor follows what was fetched, not what was handed over: otherwise a Scan stopped at the 45th row of a 50-row page would ask for the next page starting at the 46th and hand five rows over twice.
• n <= 0 sends nothing.

Count()

It was lying when a walk stopped in the middle of a page: Count() == 50 with 45 rows taken. Renaming it is not an option, so:

• Count() now counts the rows handed over, not the rows fetched. For Next that is the same number (Next hands over a whole page); for Take it is the honest one: 45, not 50.
• The other half of the problem Pager cannot fix, and does not pretend to: a break inside the caller's loop never reaches it. The godoc of Count says so outright, together with the advice to take rows with Take when their number is what matters.

Checks

• go build ./...
• go vet ./...
• gofmt -l . — no output
• go test -race ./...

Compatibility

• The change is additive: existing signatures are untouched

Count() changes meaning only where there was no value before: for a walk without Take the number is the same as it was.

Tests

pager_take_test.go — everything goes through a counter of HTTP requests, because that is exactly where it shows why Take is needed: the rows the wrong form hands over are the very same ones.

• TestTakeSpendsNoRequestOnAPageItWillNotRead — Take(45) = 1 request, the same result through a loop with break = 2. The second half of the test pins the difference itself, so that a Take which quietly brings the extra request back fails here rather than on a customer's quota.
• TestTakeStopsExactlyOnAPageBoundary — Take(50) on a page of 50 rows does not reach for a second one.
• TestTakeCrossesPagesAndStops, TestTakeReturnsShortAtTheEndOfTheList, TestTakeZeroAsksThePortalForNothing.
• TestTakeThenNextContinuesInsideTheSamePage — the rest of the page is neither lost nor re-read.
• TestCountCountsRowsHandedOverNotRowsFetched.
• TestTakeReturnsWhatItReadWhenTheWalkFails — the rows arrive alongside the error.
• TestScanTakeKeepsTheCursorOnTheFetchedPage — the cursor follows what was fetched; otherwise the export holds duplicates, and duplicates are nowhere an error, they are simply wrong.

The tests were checked with mutations: a Take implemented on top of Next with truncation, and a Next that ignores the rows not yet handed over — both fail the set.

Verified on a live portal (54 deals, HTTP requests counted on the wire by a wrapper transport):

Take(45) — 45 of the 50 rows of the first page   http requests: 1   rows=45 Count()=45
the same result through a loop with break        http requests: 2   rows=45 Count()=54
Take(45) then Next() for the rest of the page    http requests: 1   Take=45 then Next=5
Take(1000) — more than the portal holds          http requests: 2   rows=54, no error

[ExaltedTrou6]

Дополнение по ходу проверки: нашёлся и закрыт единственный цикл, которым Take мог убежать.

Сервер, отвечающий «строк нет, но продолжение есть» с двигающимся курсором, заставлял Take спрашивать бесконечно: детектор застрявшего курсора тут не срабатывает — курсор-то идёт, он просто ничего не отдаёт. В замере до правки обход успел сделать 204 запроса внутри одного вызова, из которого вызывающему выйти нечем.

zz_edge_test.go:19: Take is looping without bound   (x4)
rows=0 err=... requests=204 stalled=false

Теперь такая страница заканчивает Take с ErrCursorStalled. Разделение получилось принципиальное и оно же записано в godoc:

• Next отдаёт пустую страницу и оставляет решение вызывающему — цикл там принадлежит ему. Поведение не изменилось.
• Take владеет циклом сам, поэтому останавливается на первом признаке того, что продвинуться не может, — громко и с указанием на Next.

Закреплено тестом TestTakeStopsOnAPageThatAddsNothing: Take — один запрос и ErrCursorStalled, Next на том же сервере — по-прежнему true и nil-ошибка.