Re: Declarative partitioning - another take

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, Dmitry Ivanov <d(dot)ivanov(at)postgrespro(dot)ru>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Erik Rijkers <er(at)xs4all(dot)nl>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, pgsql-hackers-owner(at)postgresql(dot)org
Subject: Re: Declarative partitioning - another take
Date: 2016-12-22 08:35:51
Message-ID: 95da7016-2757-9b11-74f5-ad4c6f647dfb@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 2016/12/22 1:50, Robert Haas wrote:
> On Wed, Dec 21, 2016 at 5:33 AM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> Breaking changes into multiple commits/patches does not seem to work for
>> adding regression tests. So, I've combined multiple patches into a single
>> patch which is now patch 0002 in the attached set of patches.
>
> Ugh, seriously? It's fine to combine closely related bug fixes but
> not all of these are. I don't see why you can't add some regression
> tests in one patch and then add some more in the next patch.

I managed to do that this time around with the attached set of patches.
Guess I gave up too easily in the previous attempt.

While working on that, I discovered yet-another-bug having to do with the
tuple descriptor that's used as we route a tuple down a partition tree. If
attnums of given key attribute(s) are different on different levels, it
would be incorrect to use the original slot's (one passed by ExecInsert())
tuple descriptor to inspect the original slot's heap tuple, as we go down
the tree. It might cause spurious "partition not found" at some level due
to looking at incorrect field in the input tuple because of using the
wrong tuple descriptor (root table's attnums not always same as other
partitioned tables in the tree). Patch 0001 fixes that including a test.
It also addresses the problem I mentioned previously that once
tuple-routing is done, we failed to switch to a slot with the leaf
partition's tupdesc (IOW, continued to use the original slot with root
table's tupdesc causing spurious failures due to differences in attums
between the leaf partition and the root table).

Further patches 0002, 0003 and 0004 fix bugs that I sent one-big-patch for
in my previous message. Each patch has a test for the bug it's meant to fix.

Patch 0005 is the same old "Add some more tests for tuple-routing" per [1]:

> Meanwhile, committed the latest 0001 and the elog() patch.

Thanks!

Regards,
Amit

[1]
https://www.postgresql.org/message-id/CA%2BTgmoZ86v1G%2Bzx9etMiSQaBBvYMKfU-iitqZArSh5z0n8Q4cA%40mail.gmail.com

Attachment Content-Type Size
0001-Use-correct-tuple-descriptor-before-and-after-routin.patch text/x-diff 15.8 KB
0002-Make-ExecConstraints-show-the-correct-row-in-error-m.patch text/x-diff 9.6 KB
0003-Fix-a-bug-in-how-we-generate-partition-constraints.patch text/x-diff 14.2 KB
0004-Fix-a-bug-of-insertion-into-an-internal-partition.patch text/x-diff 6.7 KB
0005-Add-some-more-tests-for-tuple-routing.patch text/x-diff 5.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2016-12-22 09:56:40 Re: Proposal for CSN based snapshots
Previous Message tushar 2016-12-22 08:05:37 Re: Parallel Index Scans