| From: | Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: scary patch contest |
| Date: | 2026-08-26 21:19:24 |
| Message-ID: | CA+renyWv5kCXkOM6VkPmJ1KTmwZt2gd2JGOjstEWUOJG3uYVKA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Aug 26, 2026 at 7:29 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> > 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.
>
> What worries me here is that several of these interactions seem like fairly
> obvious ones. Not recognizing & testing some complicated interaction, race
> conditions, etc are one thing, but the oversights here seem to be pretty
> ... broad.
I agree I overlooked some feature combinations I should have
considered (e.g. WITH CHECK OPTION). But several of the security
issues come from a disagreement about whether to treat RLS the same
way the standard says to treat regular permissions: you shouldn't need
insert privilege to insert a temporal leftover, since that is just
preserving the data that is already there. I still think running RLS
checks for that will be awkward for users until we add a way for the
check to ask whether it's for a temporal leftover or not. But I will
try to be more systematic testing all feature combinations next time.
> I also wonder if we need some more scalable testing here, to test the
> combination of various interacting features. E.g. something like a perl script
> that generates the combinatorial combination of features around this being
> [not] used and verifies that results stay consistent. That could makes it much
> cheaper (in human time, not necessarily test runtime), to test some of the
> interactions here.
A while back I asked Claude to make a testing plan for how FOR PORTION
OF combines with other features. It found some untested areas and
wrote tests for them, but it didn't find any new bugs. Others have
found two new bugs since then, so this approach isn't perfect. I've
attached the testing plan. I've been meaning to send in a patch to add
the new tests, but I wanted to spend time improving them first. (The
unchecked boxes are mostly from bugs that were already known, whose
fixes and tests were not yet merged.)
This approach might be of general interest. An LLM seems better than a
deterministic approach. I don't think the relevant combinations will
be the same for any two features. Perhaps a very brief outline of all
features would be useful for an LLM to consult when writing tests for
new patches. The list we maintain should be O(n), not O(n^2). Making
it machine-readable seems worthwhile. If we really want to maintain a
physical O(n^2) list, with whether each combination is relevant,
tested, etc., that would be excellent but a lot more work. Also we
could have a generic prompt checked into the repo or put on a wiki
page.
Yours,
--
Paul ~{:-)
pj(at)illuminatedcomputing(dot)com
| Attachment | Content-Type | Size |
|---|---|---|
| for_portion_of_test_plan.md | text/markdown | 20.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2026-08-26 21:21:36 | Re: pg_restore_attribute_stats() accepts non-finite values |
| Previous Message | Sami Imseih | 2026-08-26 21:17:13 | Re: tablecmds: fix bug where index rebuild loses replica identity on partitions |