| 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 | Resend email |
| 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
| 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 |
| 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 |