Horton

Re: Declarative partitioning - another take

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Amit Langote <amitlangote09@gmail.com>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2016-12-07T14:45:58Z
Lists: pgsql-hackers 
On Wed, Dec 7, 2016 at 6:42 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2016/12/07 13:38, Robert Haas wrote:
>> On Wed, Nov 30, 2016 at 10:56 AM, Amit Langote <amitlangote09@gmail.com> wrote:
>>> The latest patch I posted earlier today has this implementation.
>>
>> I decided to try out these patches today with #define
>> CLOBBER_CACHE_ALWAYS 1 in pg_config_manual.h, which found a couple of
>> problems:
>>
>> 1. RelationClearRelation() wasn't preserving the rd_partkey, even
>> though there's plenty of code that relies on it not changing while we
>> hold a lock on the relation - in particular, transformPartitionBound.
>
> Oh, I thought an AccessExclusiveLock on the relation would prevent having
> to worry about that, but guess I'm wrong.  Perhaps, having a lock on a
> table does not preclude RelationClearRelation() resetting the table's
> relcache.

No, it sure doesn't.  The lock prevents the table from actually being
changed, so a reload will find data equivalent to what it had before,
but it doesn't prevent the backend's cache from being flushed.

>> 2. partition_bounds_equal() was using the comparator and collation for
>> partitioning column 0 to compare the datums for all partitioning
>> columns.  It's amazing this passed the regression tests.
>
> Oops, it seems that the regression tests where the above code might be
> exercised consisted only of range partition key with columns all of the
> same type: create table test(a int, b int) partition by range (a, (a+b));

It doesn't seem like it; you had this: create table part1 partition of
range_parted for values from ('a', 1) to ('a', 10);

>> I recommend that once you fix this, you run 'make check' with #define
>> CLOBBER_CACHE_ALWAYS 1 and look for other hazards.  Such mistakes are
>> easy to make with this kind of patch.
>
> With the attached latest version of the patches, I couldn't see any
> failures with a CLOBBER_CACHE_ALWAYS build.

Cool.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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.