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
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? |