Re: Unexpected (wrong?) result querying boolean partitioned table with NULL partition

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: David Kimura <david(dot)g(dot)kimura(at)gmail(dot)com>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Unexpected (wrong?) result querying boolean partitioned table with NULL partition
Date: 2023-04-13 03:45:03
Message-ID: CAApHDvr00ZjvA5WQ__OUR-cquyLhYAaxHpEbype2b-=ffrCqmw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 13 Apr 2023 at 15:30, Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> BTW, I wonder if we should elog an Error here.
>
> default:
> - Assert(false); /* hmm? */
> - return PARTCLAUSE_UNSUPPORTED;
> + elog(ERROR, "unrecognized booltesttype: %d",
> + (int) btest->booltesttype);
> + break;

I wondered about that, hence my not-so-commitable comment left in there.

My last thoughts were that maybe we should just move the IS_UNKNOWN
and IS_NOT_UNKNOWN down into the switch and let -Wall let us know if
something is missing.

It hardly seems worth keeping the slightly earlier exit for those two
cases. That just amounts to the RelabelType check and is this the
partition key. I doubt IS[_NOT]_UNKNOWN is common enough for us to
warrant contorting the code to make it a few dozen nanoseconds faster.
Having smaller code is probably more of a win, which we'd get if we
didn't add the ERROR you propose.

> Otherwise the patch looks good to me.

Thanks for having a look.

David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-04-13 03:48:14 Re: Clean up hba.c of code freeing regexps
Previous Message Amit Kapila 2023-04-13 03:34:24 Re: Should we remove vacuum_defer_cleanup_age?