scary patch contest

I asked Claude to evaluate which v19 patches were the scariest based
on the number and type of bugs fixed post-freeze. Results below, with
a few particularly cutting remarks from the LLM edited out. I also
omitted the list of runners-up that it generated, none of which seemed
to me to be of enough concern to be worth discussion. I do think these
are worth discussing, though: are any of them likely to still be in
bad enough shape that we should consider reverting them before v19
ships? Past performance is no guarantee of future results, but it's
not a contraindication either.

1. RI fast-path FK checks / batching (2da86c1ef9b, b7b27eb41a5,
Junwang Zhao and Amit Langote): ~16 fixes including an out-of-bounds
write on re-entry, a crash under nested SPI, a wrong-user-id index
scan, and five distinct classes of incorrect FK enforcement
(cross-type, domains, column order, nullable keys, non-btree indexes).
The transactional design was retrofitted post-freeze
(subtransaction/firing-cycle tracking, deferred-trigger integration,
xact.c cleanup).

2. REPACK / REPACK CONCURRENTLY (ac58465e061, 28d534e2ae0, Antonin
Houska c/b Álvaro Herrera): 28 fixes including data loss (missingvals
discarded, also affecting VACUUM FULL/CLUSTER), broken toasted-tuple
and generated-column handling, and two security-flavored ACL fixes.
Its prerequisite (database-specific logical replication snapshots,
0d3dba38c777) was reverted as fundamentally flawed, leaving v19's
REPACK CONCURRENTLY restricted to one process cluster-wide.

3. Online data checksums (f19c0eccae9, Daniel Gustafsson and Magnus
Hagander): ~25 substantive fixes, mostly state-machine holes — invalid
transitions in checkpoints and promotion, initial state missing from
the control file, basebackup false checksum failures, wrong WAL for
unlogged relations. A corruption-detection feature producing false
positives is exactly the wrong failure mode.

4. UPDATE/DELETE FOR PORTION OF (8e72d914c52, Paul Jungwirth c/b Peter
Eisentraut): 17 fixes, three of them security — leftover-row inserts
bypassed INSERT RLS policies, WITH CHECK OPTION went unenforced on
DELETE, and UPDATE permission on the range column wasn't required. The
interaction matrix (views, generated columns, inheritance, rules,
WHERE CURRENT OF, FDW partitions) had to be forbidden or fixed piece
by piece after freeze.

5. SQL/PGQ property graphs (2f094e7ac69, Peter Eisentraut and Ashutosh
Bapat): 17 fixes, but lower severity — wrong collation, unresolved
literals, broken LATERAL references, deparse and pg_dump ACL bugs,
plus a series of after-the-fact prohibitions.

6. postgres_fdw statistics import (28972b6fc3d, Corey Huinker and
Etsuro Fujita): committed on freeze day, then needed 7 fixes including
a syntax error, a quoted-column deparsing bug, an SPI removal, and a
user-facing option rename in August. High defect density for ~one file
of contrib code.

I don't currently have a firm position on what we should do here. I
think it's pretty clear that none of these were as robust at commit
time as we would like, but that doesn't mean that they're still
broken. I personally think #4 and #6 are relatively lower risk on a
going-forward basis, because they're only going to come up in fairly
niche situations. That somewhat applies to #5 as well, but it's a
bigger feature so I'm more concerned. #2 has been cut back in scope
enough that I don't think we would have accepted the feature
originally if it had been subject to those limitations, which IMHO is
a reasonable argument for a revert but also not conclusive. The same
would be true of #1 but for 268958a2e5f. I'm pretty scared about all
of #1-#3 having a long tail of bugs that we haven't found yet, in
pretty critical areas.

Thoughts?

--
Robert Haas
EDB: http://www.enterprisedb.com