| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Amit Kapila <akapila(at)postgresql(dot)org> |
| Cc: | pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: pgsql: Implement streaming mode in ReorderBuffer. |
| Date: | 2021-04-28 00:01:38 |
| Message-ID: | 2752962.1619568098@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
Amit Kapila <akapila(at)postgresql(dot)org> writes:
> Implement streaming mode in ReorderBuffer.
I notice that new buildfarm member wrasse is unhappy about some of the
code added by this commit:
"/export/home/nm/farm/studio64v12_6/HEAD/pgsql.build/../pgsql/src/backend/replication/logical/reorderbuffer.c", line 2510: Warning: Likely null pointer dereference (*(curtxn+272)): ReorderBufferProcessTXN
It's griping about this:
curtxn->concurrent_abort = true;
and I think it's got a point. There is little if any reason to have
confidence that curtxn must be non-NULL when this code is reached,
because it's in a PG_CATCH segment and there is a lot of code within
the PG_TRY that could throw an error before the spot where curtxn
is set. Not to mention that curtxn is set only conditionally.
So I think
if (curtxn)
curtxn->concurrent_abort = true;
would be cheap insurance against a core dump. Or maybe we should
do something else, but this seems really rickety as-is.
(wrasse seems to be generating boatloads of utterly useless warnings
otherwise, but after sifting through the chaff I did find this one.)
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2021-04-28 02:19:10 | pgsql: Fix pg_identify_object_as_address() with event triggers |
| Previous Message | Andrew Dunstan | 2021-04-27 12:29:00 | pgsql: Improve logic in PostgresVersion.pm |