Re: [HACKERS] Partitioned tables and relfilenode - Mailing 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