Date: Tue, 4 Sep 2018 01:08:34 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Michael Paquier <michael@paquier.xyz>,
	Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: pointless check in RelationBuildPartitionDesc
Message-ID: <20180904040834.vjmixztatyrqklp5@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <cf59a3e0-519c-dbb0-083e-db183137b893@lab.ntt.co.jp>
User-Agent: NeoMutt/20170306-137-4415bd-dirty (1.8.0)
Precedence: bulk

On 2018-Sep-04, Amit Langote wrote:

> On 2018/09/04 10:19, Michael Paquier wrote:
> > On Tue, Sep 04, 2018 at 09:47:07AM +0900, Amit Langote wrote:
> >> On 2018/09/04 6:39, Alvaro Herrera wrote:
> >>> After looking, it seems that this is just self-inflicted pain: for some
> >>> reason, we store the pg_inherits row for a partition, and immediately
> >>> afterwards compute and store its partition bound, which requires the
> >>> above hack.  But if we do things in the opposite order, this is no
> >>> longer needed.  I propose to remove it, as in the attached patch.
> >>
> >> +1.  I remember having facepalmed at this before and had also written a
> >> patch but never got around to submitting it.
> > 
> > Ok, I see.  It seems to me that this could be replaced by an
> > elog(ERROR), as relispartition ought to be set anyway.  This way any
> > future callers would get things done in the correct order.
> 
> Converting it to elog(ERROR, ...) might be a good idea.

I think it'd be pointless noise.  If we really want to protect against
that, I think we should promote the Assert for relpartbound's isnull
flag into an if test.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services