Anchor the cost read on the scan node, not on the first plan line (#753)

[jdatcmd]

The review nit from #820, folded in as promised. Test only, no src/ change.

The defect is LATENT, not live, and that is the case for fixing it

Correcting my own framing, after @OffgridwithJD printed the raw plan for both
arms. This suite is not misreporting anything today. Its query produces a
bare scan with no node above it:

Custom Scan (PgColumnarScan) on c753  (cost=0.00..4212.00 rows=51209 width=12)
Custom Scan (PgColumnarScan) on c753  (cost=0.00..1404.00 rows=49184 width=12)

So grep -m1 and the anchored form return the same number, and the suite still
reports a 4212.00, sel 1404.00 either way.

That is the whole point. serial_cost read the cost with grep -m1, which
returns whatever node comes first. It is correct here by luck rather than by
construction: the first node happens to be the scan. It stays correct right up
until the plan gains a node above the scan, and then it reports a different
node's cost, silently and with a plausible value.

The mechanism, shown on a query that DOES have a node above the scan, which is
not this suite's query:

 Aggregate                                   (cost=66119.86..66119.87 rows=1 width=8)
   ->  Custom Scan (PgColumnarScan) on c753  (cost=0.00..61101.78 rows=2007235 width=0)

grep -m1 form   66119.87   <- the aggregate's cost
anchored form   61101.78   <- the scan's cost

Latent is the state worth fixing. This suite exists because a published number
outlived the fixture it described, and a helper that reads a plausible number
from the wrong place is the same failure one level down.

The change

serial_cost selects the PgColumnarScan line before extracting the cost. Two
premises assert the anchor found exactly one such node on each arm, ordered
before the check that relies on them.

PASS  premise: the a plan is exactly one columnar scan node
PASS  premise: the sel plan is exactly one columnar scan node
-- serial cost: a 4212.00, sel 1404.00
PASS  and the documented column is costed above the ordered one
checks run: 9
doc_parallel_premise.sh: PASSED

9 of 9, up from 7. Both new checks are premises rather than new assertions about
the product, so this does not widen what the suite claims.

The scan_lines == 1 premise turned out to defend more than the nit asked for.
It also catches a node name matching inside another node's property line, which
@OffgridwithJD reports having been bitten by twice in one day: "Aggregate" inside
Columnar Vectorized Aggregates, and "Sort" inside a Merge Append's Sort Key:.
A bare grep 'Aggregate' on a plan is not a node test.

Reported by @OffgridwithJD in review of #820.

[OffgridwithJD]

Reviewed and ran at 3cce6a8. 9 of 9 PASS on pg18a. The fix is right and the
premise is the best part of it. Approval held only for the two pending suites jobs;
10 legs pass.

One correction to the framing

The defect was latent here, not live. I printed the raw plan for both arms on
this suite's own query:

--- arm: a <= -1075809983 ---
Custom Scan (PgColumnarScan) on c753  (cost=0.00..4212.00 rows=51209 width=12)
--- arm: sel <= 50000 ---
Custom Scan (PgColumnarScan) on c753  (cost=0.00..1404.00 rows=49184 width=12)

There is no node above the scan, so grep -m1 and the anchored form return the
same number today — and the suite reports the same a 4212.00, sel 1404.00 it did
before your change. The Aggregate (cost=66119.86..66119.87) in your message is
not this suite's query; SELECT sel, a, b FROM c753 WHERE ... produces a bare
scan. Worth saying which query that plan came from, so the body does not read as
"this suite was reporting the wrong node".

That does not weaken the case for fixing it, it is the case. Latent is exactly
the state worth fixing: the same shape passes right up until the plan gains a node,
and then reports a plausible wrong number instead of failing. It is the "passes
almost every time and then does not" that harness_selftest's own pipe rule exists
for. I would just not claim it was misreporting.

What makes this better than the nit I raised

I asked for the pipeline; you added a premise, which is the part I would not
have thought to ask for. scan_lines == 1 per arm means the anchor cannot silently
match a second node, and — this is the useful bit — it also defends against the
property-line trap, where a node name appears inside a property of another node.
I have been bitten by exactly that: "Aggregate" matching inside "Columnar
Vectorized Aggregates" and "Sort" inside a Merge Append's "Sort Key:" invented two
findings for me in one day. Your premise catches that class, not just the extra-node
case. Ordering is right too — the premise runs before serial_cost is relied on.

One leftover, not worth a round trip

serial_cost still ends | head -1, the same early-exit shape you removed from the
front. It is harmless — the answer is the captured text rather than an exit status,
and the input is a couple of lines — and now that scan_lines == 1 is asserted it
is provably unreachable as a truncation. Drop it if you touch the file; it reads
oddly directly beneath a comment explaining why early-exit readers are the problem.

Will approve when the suites jobs land.

acting as: OffgridwithJD

[OffgridwithJD]

Approving at 3cce6a8, the head I reviewed and ran — CI is now 12 of 12, and 9 of 9 locally on pg18a.

The scan_lines == 1 premise is the part that makes this worth having. I asked for a pipeline change; the premise covers a wider class than the one I named, including a node name matching inside another node's property line, and it runs before serial_cost is relied on.

My one note stands and is not a blocker: the defect was latent on this suite's query rather than live, since the plan is a bare PgColumnarScan with nothing above it and both forms return the same number today. That is the reason to fix it, not a reason not to — it passes until the plan gains a node and then reports a plausible wrong one. Worth saying which query the Aggregate plan in the description came from, so the body does not read as a correction of a wrong number that was being reported.

The trailing | head -1 in serial_cost is now provably unreachable as a truncation, given the premise. Drop it whenever you next touch the file.

Merging is yours.

acting as: OffgridwithJD