Date: Sun, 20 Mar 2022 18:58:07 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Zhihong Yu <zyu@yugabyte.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Andrew Dunstan <andrew@dunslane.net>,
	Ibrar Ahmed <ibrar.ahmad@gmail.com>,
	Arne Roland <A.Roland@index.de>,
	"David G. Johnston" <david.g.johnston@gmail.com>,
	Masahiko Sawada <sawada.mshk@gmail.com>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Rahila Syed <rahilasyed90@gmail.com>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>
Subject: Re: a misbehavior of partition row movement (?)
Message-ID: <202203201758.kzmjeptr2c5f@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+HiwqHPVedqaXO3S9iWRbN6j6sLD94i_GVNeTCOWsvsqsNx+w@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202203201758.kzmjeptr2c5f%40alvherre.pgsql>
Precedence: bulk

On 2022-Mar-20, Amit Langote wrote:

> On Sun, Mar 20, 2022 at 5:13 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > On 2022-Mar-18, Zhihong Yu wrote:

> > > +   if (!partRel->rd_rel->relispartition)
> > > +       elog(ERROR, "cannot find ancestors of a non-partition result
> > > relation");
> > >
> > > It would be better to include the relation name in the error message.
> >
> > I don't think it matters.  We don't really expect to hit this.
> 
> I tend to think maybe showing at least the OID in the error message
> doesn't hurt, but maybe we don't need to.

Since we don't even know of a situation in which this error message
would be raised, I'm hardly bothered by failing to print the OID.  If
any users complain, we can add more detail.

> I've fixed the comment as:
> 
> -       /* Ignore the root ancestor, because ...?? */
> +       /* Root ancestor's triggers will be processed. */

Okay, included that.

> A description of what we're looking for with this code is in the
> comment above the loop:
> 
>     /*
>      * For any foreign keys that point directly into a non-root ancestors of
>      * the source partition,...
> 
> So finding triggers in those non-root ancestors whose function is
> RI_TRIGGER_PK tells us that those relations have foreign keys pointing
> into it or that it is the PK table in that relationship.  Other than
> the comment, the code itself seems self-documenting with regard to
> what's being done (given the function/macro/variable naming), so maybe
> we're better off without additional commentary here.

Yeah, WFM.

> I've attached a delta patch on v16 for the above comment and a couple
> of other changes.

Merged that in, and pushed.  I made a couple of wording changes in
comments here and there as well.

I lament the fact that this fix is not going to hit Postgres 12-14, but
ratio of effort to reward seems a bit too high.  I think we could
backpatch the two involved commits if someone is motivated enough to
verify everything and come up with solutions for the necessary ABI
changes.

Thank you, Amit, for your perseverance in getting this bug fixed!  

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
"No hay hombre que no aspire a la plenitud, es decir,
la suma de experiencias de que un hombre es capaz"