On Mon, Jan 17, 2022 at 6:26 AM Alvaro Herrera <alvhe...@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
