From: Tom Lane <tgl@sss.pgh.pa.us>
To: jesper.pedersen@redhat.com
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: sulamul@gmail.com, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Subject: Re: COLLATE: Hash partition vs UPDATE
Date: Sun, 14 Apr 2019 16:50:07 -0400
Message-ID: <30551.1555275007@sss.pgh.pa.us> (raw)
In-Reply-To: <826e4d52-234a-4d35-73b2-7c6b0a92220e@redhat.com>
References: <a620f85a-42ab-e0f3-3337-b04b97e2e2f5@redhat.com>
<b462ff0c-a846-dce6-b0a7-ab1397e73b98@lab.ntt.co.jp>
<826e4d52-234a-4d35-73b2-7c6b0a92220e@redhat.com>
Jesper Pedersen <jesper.pedersen@redhat.com> writes:
> Yeah, that works here - apart from an issue with the test case; fixed in
> the attached.
Couple issues spotted in an eyeball review of that:
* There is code that supposes that partsupfunc[] is the last
field of ColumnsHashData, eg
fcinfo->flinfo->fn_extra =
MemoryContextAllocZero(fcinfo->flinfo->fn_mcxt,
offsetof(ColumnsHashData, partsupfunc) +
sizeof(FmgrInfo) * nargs);
I'm a bit surprised that this patch manages to run without crashing,
because this would certainly not allocate space for partcollid[].
I think we would likely be well advised to do
- FmgrInfo partsupfunc[PARTITION_MAX_KEYS];
+ FmgrInfo partsupfunc[FLEXIBLE_ARRAY_MEMBER];
to make it more obvious that that has to be the last field. Or else
drop the cuteness with variable-size allocations of ColumnsHashData.
FmgrInfo is only 48 bytes, I'm not really sure that it's worth the
risk of bugs to "optimize" this.
* I see collation-less calls of the partsupfunc at both partbounds.c:2931
and partbounds.c:2970, but this patch touches only the first one. How
can that be right?
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, jesper.pedersen@redhat.com, Langote_Amit_f8@lab.ntt.co.jp, pgsql-hackers@lists.postgresql.org, peter.eisentraut@2ndquadrant.com
Subject: Re: COLLATE: Hash partition vs UPDATE
In-Reply-To: <30551.1555275007@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox