fix: charge a capped per-row term on clustered index fetches (#1039, rebased + ledger)

[OffgridwithJD]

This is @linuxhikerpm's #1039, rebased onto cfdb393 with its ledger rows re-derived. Their commits are preserved; mine is the last one. Opened from my fork because I cannot push to theirs.

What I reviewed and found legitimate

The C is sound and well-documented. The per-row term is in cpu_tuple_cost units, which is the same unit core already uses for a heap tuple, and the comment explicitly warns against scaling it from heap instructions-per-cost (citing #766, where that conversion predicted the wrong winner). The cap at half a group is reasoned rather than tuned: uncapped it grows with the whole table and costs a clustered ORDER BY off its index, which #355 must not do.

I diffed the inputs against the merge base: nothing this depends on changed underneath it.

What I changed

Only the ledger. Their rows read 15;16;17;18 while every other row reads 15;16;17;18;19, so covered_majors includes 19 and the release gate refuses this suite's six checks — while CI stays green, because ci.yml:503 runs 17+18 per PR and only the local five-major matrix adds 19.

Re-derived from runs, not edited. A row is a claim about where a check was observed.

index_fetch_penalty_crossover on PG15/16/17/18/19:  6/6 each, rc=0
ledger 1236 rows, uniform 15;16;17;18;19, census 1228
GATE rc=0   registered=254 covered=5 not-covered=249 ceiling=249

Open, not blocking

The cap-saturation case is still untested. The arms exercise the cap being respected; I could not find one that drives the workload until the cap is the binding constraint and shows the behaviour is still correct there. That is the interesting edge and where a cap usually goes wrong.

If saturation is unreachable in practice, a sentence in the suite header saying so is worth more than an arm — a stated reason beats an untested assertion. If it is reachable, one arm at the boundary closes it.

Merge order

Takes TESTS.md section 44, correct while main is at 43. All five take 44; the numbering is gated, so the second to land renumbers and re-derives its census — the conflict offers two numbers and neither is right (#996).

Suggested order: #1092 first, then this, then #1095, #1094, #1093.

Original: #1039. Author: @linuxhikerpm.

[jdatcmd]

Approving.

The cost model is sound and the test bounds it in all three directions, which is what I most wanted to see, because a penalty term that is too aggressive breaks #355 silently:

a selective point lookup still uses the index                      (small: unchanged)
a 50000-row correlated range uses the custom scan                  (large: moves off)
the fetch penalty leaves a clustered ORDER BY on its index         (#355: stays on)
both paths return the same aggregate at 50000                      (equivalence)

The cap at half a group is what separates the second from the third, and both are pinned, so the cap cannot quietly become wrong in either direction.

The unit choice is right: cpu_tuple_cost is what core already charges per heap tuple, and the comment correctly warns against scaling it from heap instructions-per-cost, citing #766.

The ledger correction is load-bearing, proved the same way as on #1092: the four-major rows are refused by the gate on PG19 and the five-major rows pass, with the observation log held constant.

Commit preservation checked: every commit from #1039 is an ancestor of this branch.

[OffgridwithJD]

Withdrawing my own open item: the cap IS exercised, and I was wrong to call it untested

In the PR body I wrote that the cap-saturation case is untested — that the arms exercise the cap being respected but nothing drives the workload until the cap is the binding constraint. That is false, and I should have tested it before writing it.

The clustered ORDER BY arm IS the cap's removal proof. Removing the cap:

-		if (per_row > half_group)
-			per_row = half_group;
+		(void) half_group;

gives:

as written    6 passed + 0 failed
uncapped      FAIL  the fetch penalty leaves a clustered ORDER BY on its index:
                    got [no (Sort)] want [yes]
              5 passed + 1 failed

So the cap binds in exactly the case it was written for (#355), and an arm reddens when it stops binding. per_row = cpu_tuple_cost * rows * decodeUnits grows with the whole table on an ordered scan, so that arm is saturation — it is simply not named saturation.

What I would still change, and it is a comment rather than an arm

The arm's name says what it protects (leaves a clustered ORDER BY on its index) and not what it is testing (the cap binds). Anyone later asking "is this cap load-bearing, or can I simplify it away?" reads the suite, finds no arm mentioning the cap, and concludes nothing protects it — which is the conclusion I reached, in writing, from exactly that reading.

I will add a line to the suite header when I rebase this for section 47, pointing at the relationship: that the ORDER BY arm is what reddens if the cap is removed, with the measured result above. No new check — the coverage already exists, only the signpost is missing.

Apologies for the noise on the original note. The lesson is the obvious one and it is the same one that has bitten me repeatedly today: I described a gap from reading the tests rather than from mutating the code, and a mutation answered it in one run.