MIME-Version: 1.0
References: 
 <CAJ7c6TME+ix6VRf-2TPnVTsj8qn_hy6sYAOmMhZEivwsu2wS6g@mail.gmail.com>
 <626986.1776785090@sss.pgh.pa.us>
 <CACJufxFgKVL=NnPocAwhRFqpLHckN3N1Ndf7FjGNTdtsAA26mg@mail.gmail.com>
 <CA+renyW52FJ=+pnh+N_xqjUev_MNUCk_mgD2UC7UT0jyAHCE8Q@mail.gmail.com>
 <84f20d0d-3bc8-4843-bd7e-21bce20f8582@eisentraut.org>
 <CAJ7c6TO+kTwk5MBayLARxsPk-YCsDt2QH9P_PeTYoc3zPYCS3Q@mail.gmail.com>
 <CA+renyU6UZnpkgAuog0BVhqyD=G+_UVLzN2Ccpt2sV-qkF-_VQ@mail.gmail.com>
 <CAJ7c6TN7dekEHU8Vu4jV5+Zq7h2cVZQgwkYFCrU0jrjT26-ANw@mail.gmail.com>
 <CA+renyXs5mkPb-Vg=uF2Hj0jP2ufutc1_Eht=eb3+b_WpjiqLg@mail.gmail.com>
 <4984118e-0dc5-49a9-b279-fd0b707be8e0@eisentraut.org>
 <CA+renyUkKaWtWZX2UcojKVjM=f1F5kgdvR8Pr5QqyJqqe_dw-g@mail.gmail.com>
 <391a2845-105b-4048-a113-835c2742fea1@eisentraut.org>
 <CA+renyXPEJYOVov3XtzTp3NusGkObc9UeW-Nm78K8_nsWSnnfg@mail.gmail.com>
 <1894792.1783476457@sss.pgh.pa.us>
 <CA+renyV1WUtDSOHpNVv3fhD6JqhzZY_mV-0Nks23MczXLxpE8A@mail.gmail.com>
 <82f35e5c-9983-4c97-8567-451bc2cb8761@eisentraut.org>
In-Reply-To: <82f35e5c-9983-4c97-8567-451bc2cb8761@eisentraut.org>
From: Paul A Jungwirth <pj@illuminatedcomputing.com>
Date: Thu, 9 Jul 2026 08:22:38 -0700
Message-ID: 
 <CA+renyWb-253nMz4OXPwn5ebXF7oJqT2Wu=qrGv=awYFqOk2LA@mail.gmail.com>
Subject: Re: [PATCH] Fix null pointer dereference in PG19
To: Peter Eisentraut <peter@eisentraut.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
 Aleksander Alekseev <aleksander@tigerdata.com>,
	PostgreSQL Development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BrenyWb-253nMz4OXPwn5ebXF7oJqT2Wu%3DqrGv%3DawYFqOk2LA%40mail.gmail.com>
Precedence: bulk

On Thu, Jul 9, 2026 at 1:32=E2=80=AFAM Peter Eisentraut <peter@eisentraut.o=
rg> wrote:
>
> > Here is another patch series. No code changes, but I inserted a new
> > patch with tests showing that parse-time checking crashes, but
> > rewrite-time and exec-time checking catches the forbidden statement.
> > So the series is:
> >
> > v1: parse-time check, tests pass
> > v2: add tests that crash the server
> > v3: rewrite-time check: tests pass
> > v4: exec-time check: tests pass
>
> What do you want to do with these?  Here you list them as four different
> versions, but the attachments are four incremental patches of the same
> version.  Do you propose to apply all of them?
>
> It seems we definitely do want the exec-time check.  And I can see maybe
> the parse-time check as an additional user-friendliness feature.  But
> maybe we don't need to check three times?

I should have called them part{1-4} instead of v{1-4}. My intent is to
squash them, but I thought separating the changes clarified the
problem and how v3/v4 fix it. Either squash v1-3 if you want to check
at rewrite time, or v1-4 if you want to check at exec time.

Each part removes the check from the prior part, so there is only one
check, it's just a question of when we do it.

Checking at v1 is Aleksander's original patch. Then v2 adds tests that
crash the server. v3 moves the check to rewrite time, and the tests
pass. v4 moves the check again to exec time. But v4 is a behavior
change: an UPDATE that touches zero rows no longer fails. I thought
that was weird; that's why I recommended leaving it out (in
https://www.postgresql.org/message-id/CA%2BrenyXPEJYOVov3XtzTp3NusGkObc9UeW=
-Nm78K8_nsWSnnfg%40mail.gmail.com
before I added the crashing tests).

I like your idea of keeping the parse-time check too. Then we can show
line numbers. Want a patch with that?

If we check at both rewrite-time and exec-time, the exec-time check
will never happen (as far as I can tell). If we want something there,
maybe it should be an Assert or an elog.

I can give you one squashed file if you like. After reading your
email, my preference is to check at parse time, check again at rewrite
time, and Assert at exec time.

Yours,

--=20
Paul              ~{:-)
pj@illuminatedcomputing.com