Re: a misbehavior of partition row movement (?) - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: a misbehavior of partition row movement (?)
Date
Msg-id CALNJ-vQgAroyjsKi4=2UFQiU+NGMNo7GQ1-VO5v_LGNke3GmWg@mail.gmail.com
Whole thread Raw
In response to Re: a misbehavior of partition row movement (?)  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: a misbehavior of partition row movement (?)
List pgsql-hackers


On Mon, Jan 17, 2022 at 6:26 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2022-Jan-17, Amit Langote wrote:

> Note that the fix involves adding fields to ResultRelInfo -- v13 needs
> 2 additional, while v14 and HEAD need 1.  That combined with needing
> new catalog entries for parent FK triggers, back-patching this does
> make me a bit uncomfortable.

Yeah, that's a good point, so I ran abidiff on the binaries in branch 13
to have some data on it.  The report does indeed have a lot of noise
about those three added members in struct ResultRelInfo; but as far as I
can see in the report, there is no ABI affected because of these
changes.

However, the ones that caught my eye next were the ABI changes for
ExecGetTriggerResultRel() and ExecAR{Delete,Update}Triggers().  These seem more
significant, if any external code is calling these.  Now, while I think
we could dodge that (at least part of it) by having a shim for
AfterTriggerSaveEvent that passes a NULL mtstate, and takes the
assumption that there is no row partition migration when that happens
... that seems like treading in dangerous territory: we would have
code that would behave differently for an extension that was compiled
with an earlier copy of the backend.

So I see two options.  One is to introduce the aforementioned shim, but
instead of making any assumptions, we cause the shim raise an error:
"your extension is outdated, please recompile with the new postgres
version".  However, that seems even more harmful, because production
systems that auto-update to the next Postgres version would start to
fail.

The other is suggested by you:

> Another thing to consider is that we haven't seen many reports of the
> problem (UPDATEs of partitioned PK tables causing DELETEs in
> referencing tables), even though it can be possibly very surprising to
> those who do run into it.

Do nothing in the old branches.


Another thing I saw which surprised me very much is this bit, which I
think must be a bug in abidiff:

                                type of 'EPQState* EState::es_epq_active' changed:
                                  in pointed to type 'struct EPQState' at execnodes.h:1104:1:
                                    type size hasn't changed
                                    1 data member changes (3 filtered):
                                      type of 'PlanState* EPQState::recheckplanstate' changed:
                                        in pointed to type 'struct PlanState' at execnodes.h:1056:1:
                                          entity changed from 'struct PlanState' to compatible type 'typedef PlanState' at execnodes.h:1056:1

Hi,
I think option 2, not backpatching, is more desirable at this stage.

If, complaints from the field arise in the future, we can consider backpatching.

Cheers

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Refactoring of compression options in pg_basebackup
Next
From: Andrew Dunstan
Date:
Subject: Re: Adding CI to our tree