Re: pointless check in RelationBuildPartitionDesc

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pointless check in RelationBuildPartitionDesc
Date: 2018-09-04 12:51:28
Message-ID: 20180904125128.ntliwrygsylaar4b@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2018-Sep-04, Amit Langote wrote:

> On 2018/09/04 13:08, Alvaro Herrera wrote:

> > 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.
>
> So that we can elog(ERROR, ...) if isnull is true? If so, I agree,
> because that's what should've been done here anyway (for a value read from
> the disk that is).

Yes.

I wonder now what's the value of using an Assert for this, BTW: if those
are enabled, then we'll crash saying "this column is null and shouldn't!".
If they are disabled, we'll instead crash (possibly in production)
trying to decode the field. What was achieved?

With an elog(ERROR) the reaction is the expected one: the current
transaction is aborted, but the server continues to run.

> I think we should check relispartition then too.

Well, we don't check every single catalog flag in every possible code
path just to ensure it's sane. I don't see any reason to do differently
here. We rely heavily on the catalog contents to be correct, and
install defenses against user-induced corruption that would cause us to
crash; but going much further than that would be excessive IMO.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2018-09-04 13:00:51 Re: [HACKERS] proposal: schema variables
Previous Message Kyotaro HORIGUCHI 2018-09-04 10:52:50 Re: [HACKERS] Restricting maximum keep segments by repslots