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
