Horton

Re: Declarative partitioning - another take

Amit Langote <langote_amit_f8@lab.ntt.co.jp>

View thread View on postgr.es
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, Robert Haas <robertmhaas@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2016-08-17T06:21:07Z
Lists: pgsql-hackers 
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.

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

Commits

  1. Fix typo.

  2. Document trigger-firing behavior for inheritance/partitioning.

  3. Fire per-statement triggers on partitioned tables.

  4. Set ecxt_scantuple correctly for tuple routing.

  5. Fix interaction of partitioned tables with BulkInsertState.

  6. Avoid core dump for empty prepared statement in an aborted transaction.

  7. Fix some problems in check_new_partition_bound().

  8. Remove unnecessary arguments from partitioning functions.

  9. Fix reporting of constraint violations for table partitioning.

  10. Fix tuple routing in cases where tuple descriptors don't match.

  11. Invalid parent's relcache after CREATE TABLE .. PARTITION OF.

  12. Doc: improve documentation about inheritance.