Re: Declarative partitioning - another take

On Tue, Dec 13, 2016 at 1:58 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Attaching the above patch, along with some other patches posted earlier,
> and one more patch fixing another bug I found.  Patch descriptions follow:
>
> 0001-Miscallaneous-code-and-comment-improvements.patch
>
> Fixes some obsolete comments while improving others.  Also, implements
> some of Tomas Vondra's review comments.

Committed with some pgindenting.

> 0002-Miscallaneous-documentation-improvements.patch
>
> Fixes inconsistencies and improves some examples in the documentation.
> Also, mentions the limitation regarding row movement.

Ignored because I committed what I think is the same or a similar
patch earlier.  Please resubmit any remaining changes.

> 0003-Invalidate-the-parent-s-relcache-after-partition-cre.patch
>
> Fixes a bug reported by Tomas, whereby a parent's relcache was not
> invalidated after creation of a new partition using CREATE TABLE PARTITION
> OF.  This resulted in tuple-routing code not being to able to find a
> partition that was created by the last command in a given transaction.

Shouldn't StorePartitionBound() be responsible for issuing its own
invalidations, as StorePartitionKey() already is?  Maybe you'd need to
pass "parent" as another argument, but that way you know you don't
have the same bug at some other place where the function is called.

> 0004-Fix-a-bug-of-insertion-into-an-internal-partition.patch
>
> Fixes a bug I found this morning, whereby an internal partition's
> constraint would not be enforced if it is targeted directly.  See example
> below:
>
> create table p (a int, b char) partition by range (a);
> create table p1 partition of p for values from (1) to (10) partition by
> list (b);
> create table p1a partition of p1 for values in ('a');
> insert into p1 values (0, 'a');  -- should fail, but doesn't

I expect I'm missing something here, but why do we need to hoist
RelationGetPartitionQual() out of InitResultRelInfo() instead of just
having BeginCopy() pass true instead of false?

(Also needs a rebase due to the pgindent cleanup.)

> 0005-Fix-a-tuple-routing-bug-in-multi-level-partitioned-t.patch
>
> Fixes a bug discovered by Dmitry Ivanov, whereby wrong indexes were
> assigned to the partitions of lower levels (level > 1), causing spurious
> "partition not found" error as demonstrated in his email [1].

Committed.  It might have been good to include a test case.

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