From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
Cc: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Partition-wise join for join between (declaratively) partitioned tables |
Date: | 2017-10-03 02:18:11 |
Message-ID: | CA+Tgmoa1icxn-+XMT1HGm1ogY6z8gcYqfrOAfbyyjp-RsaaY3A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 21, 2017 at 8:21 AM, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> Here's set of rebased patches. The patch with extra tests is not for
> committing. All other patches, except the last one, will need to be
> committed together. The last patch may be committed along with other
> patches or as a separate patch.
In set_append_rel_size, is it necessary to set attr_needed =
bms_copy(rel->attr_needed[index]) rather than just pointing to the
existing value? If so, perhaps the comments should explain the
reasons. I would have thought that the values wouldn't change after
this point, in which case it might not be necessary to copy them.
Regarding nomenclature and my previous griping about wisdom, I was
wondering about just calling this a "partition join" like you have in
the regression test. So the GUC would be enable_partition_join, you'd
have generate_partition_join_paths(), etc. Basically just delete
"wise" throughout.
The elog(DEBUG3) in try_partition_wise_join() doesn't follow message
style guidelines and I think should just be removed. It was useful
for development, I'm sure, but it's time for it to go.
+ elog(ERROR, "unrecognized path node type %d", (int) nodeTag(path));
I think we should use the same formulation as elsewhere, namely
"unrecognized node type: %d". And likewise probably "unexpected join
type: %d".
partition_join_extras.sql has a bunch of whitespace damage, although
it doesn't really matter since, as you say, that's not for commit.
(This is not a full review, just a few thoughts.)
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2017-10-03 02:48:43 | Re: [PATCH] Tests for reloptions |
Previous Message | Michael Paquier | 2017-10-03 01:48:41 | Re: [PATCH] Assert that the correct locks are held when calling PageGetLSN() |