Re: [HACKERS] Partitioned tables and relfilenode - Mailing list pgsql-hackers
From | Ashutosh Bapat |
---|---|
Subject | Re: [HACKERS] Partitioned tables and relfilenode |
Date | |
Msg-id | CAFjFpRfD-bh=3iO+4Vu7CRQbe+5AAJTqyckeQDOR8ymj-UPLkg@mail.gmail.com Whole thread Raw |
In response to | Re: [HACKERS] Partitioned tables and relfilenode (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>) |
Responses |
Re: [HACKERS] Partitioned tables and relfilenode
|
List | pgsql-hackers |
> >> I am wondering whether we should deal with inh flat reset in a >> slightly different way. Let expand_inherited_rtentry() mark inh = >> false for the partitioned tables without any partitions and deal with >> those at the time of estimating size by marking those as dummy. That >> might be better than multiple changes here. I will try to cook up a >> patch soon for that. > > Are thinking something along the lines of the attached rewritten patch > 0003? I also tend to think that's probably a cleaner patch. Thanks for > the idea. Yes. Thanks for working on it. > >> Also we should add tests to make sure the scans on partitioned tables >> without any partitions do not get into problems. PFA patch which adds >> those tests. > > I added the test case you suggest, but kept just the first one. The second one was testing multi-level partitioning case, which deserves a testcase by its own. Some more comments The comment below seems too verbose. All it can say is "A partitioned table without any partitions results in a dummy relation." I don't think we need to be explicit about rte->inh. But it's more of personal preference. We will leave it to the committer, if you don't agree. + /* + * An empty partitioned table, i.e., one without any leaf + * partitions, as signaled by rte->inh being set to false + * by the prep phase (see expand_inherited_rtentry). + */ We don't need this comment as well. Rather than repeating what's been said at the top of the function, it should just refer to it like "nominal relation has been set above for partitioned tables. For other parent relations, we'll use the first child ...". + * + * If the parent is a partitioned table, we do not have a separate RTE + * representing it as a member of the inheritance set, because we do + * not create a scan plan for it. As mentioned at the top of this + * function, we make the parent RTE itself the nominal relation. */ Following condition is not very readable. It's not evident that it's of the form (A && B) || C, at a glance it looks like it's A && (B || C). + if ((rte->relkind != RELKIND_PARTITIONED_TABLE && + list_length(appinfos) < 2) || list_length(appinfos) < 1) Instead you may rearrage it as min_child_rels = (rte->relkind == RELKIND_PARTITIONED_TABLE ? 1 : 2); if (list_length(appinfos) < min_child_rels) -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
pgsql-hackers by date: