On Wed, Aug 17, 2016 at 11:51 AM, Amit Langote <
langote_amit...@lab.ntt.co.jp> wrote:
> On 2016/08/17 14:33, Ashutosh Bapat wrote:
> >> +relid_is_partition(Oid relid)
> >> +{
> >> + return SearchSysCacheExists1(PARTRELID,
> ObjectIdGetDatum(relid));
> >> +}
> >>
> >> This is used in a lot of places, and the overhead of checking it in
> >> all of those places is not necessarily nil. Syscache lookups aren't
> >> free. What if we didn't create a new catalog for this and instead
> >> just added a relpartitionbound attribute to pg_class? It seems a bit
> >> silly to have a whole extra catalog to store one extra column...
> >>
> > It looks like in most of the places where this function is called it's
> > using relid_is_partition(RelationGetRelid(rel)). Instead probably we
> should
> > check existence of rd_partdesc or rd_partkey within Relation() and make
> > sure that those members are always set for a partitioned table. That will
> > avoid cache lookup and may give better performance.
>
> It seems you are talking about a *partitioned* relation here, whereas
> relid_is_partition() is to trying to check if a relation is *partition* by
> looking up the pg_partition catalog (or the associated cache). For the
> former, the test you suggest or rd_rel->relkind ==
> RELKIND_PARTITIONED_TABLE test is enough.
>
Uh, you are right. Sorry for my misunderstanding.
>
> I am slightly tempted to eliminate the pg_partition catalog and associated
> syscache altogether and add a column to pg_class as Robert suggested.
> That way, all relid_is_partition() calls will be replaced by
> rel->rd_partbound != NULL check. But one potential problem with that
> approach is that now whenever a parent relation is opened, all the
> partition relations must be opened to get the partbound value (to form the
> PartitionDesc to be stored in parent relation's rd_partdesc). Whereas
> currently, we just look up the pg_partition catalog (or the associated
> cache) for every partition and that gets us the partbound.
>
> > That brings up another question. Can we have rd_partdesc non null and
> > rd_partkey null or vice-versa. If not, should we club those into a single
> > structure like Partition (similar to Relation)?
>
> It's true that rd_partkey and rd_partdesc are both either NULL or
> non-NULL, so combining them into a single struct is an idea worth
> considering.
>
> Thanks,
> Amit
>
>
>
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
