Re: scary patch contest

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: scary patch contest
Date: 2026-08-26 06:53:47
Message-ID: CAA4eK1Jy3yt4dMv6iAc-Peuk6wZHiusBhDZ39WM1AQQeTFJWDA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 25, 2026 at 10:18 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> 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.
>

I agree with your judgement about #4 and #6. For others, I don't know
based on the current number and severity of issues how we should
decide whether to revert or not because it is quite possible that this
time we found more bugs early with the help of AI. If we are concerned
about their quality, one possibility is to do a post-review of some of
those.

>
#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,
>

Yeah, this is a valid point. But in favor of this feature, we had
patches for the current limitations but they were not mature enough to
be committed or released. I hope that we will work on removing these
limitations in future releases. As per my knowledge, we had a
consensus on the design for the current committed work and the same
has been reviewed as well. Sure, we had found more bugs than is
normally expected but that could be both due to AI and the feature is
committed near to feature-freeze. I vote to keep #2.

--
With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2026-08-26 07:01:30 Re: Assertion failure in GetSubscriptionRelations() with concurrent DROP TABLE
Previous Message Ewan Young 2026-08-26 06:29:41 Re: right() returns the whole string for the most negative n