Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Hi Justin,

Thank you for your review. Please check v9 of the patchset [1].

On Wed, Apr 24, 2024 at 11:26 PM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
> This patch also/already fixes the schema issue I reported. Thanks.
>
> If you wanted to include a test case for that:
>
> begin;
> CREATE SCHEMA s;
> CREATE SCHEMA t;
> CREATE TABLE p(i int) PARTITION BY RANGE(i);
> CREATE TABLE s.c1 PARTITION OF p FOR VALUES FROM (1)TO(2);
> CREATE TABLE s.c2 PARTITION OF p FOR VALUES FROM (2)TO(3);
> ALTER TABLE p MERGE PARTITIONS (s.c1, s.c2) INTO s.c1; -- misbehaves if merging into the same name as an existing partition
> \d+ p
> ...
> Partitions: c1 FOR VALUES FROM (1) TO (3)

There is already a test which checks merging into the same name as an
existing partition. And there are tests with schema-qualified names.
I'm not yet convinced we need a test with both these properties
together.

> > 0002
> > The persistence of the new partition is copied as suggested in [1].
> > But the checks are in-place, because search_path could influence new
> > table persistence. Per review [2], commit message typos are fixed,
> > documentation is revised, revised tests to cover schema-qualification,
> > usage of search_path.
>
> Subject: [PATCH v8 2/7] Make new partitions with parent's persistence during MERGE/SPLIT operations
>
> This patch adds documentation saying:
> + Any indexes, constraints and user-defined row-level triggers that exist
> + in the parent table are cloned on new partitions [...]
>
> Which is good to say, and addresses part of my message [0]
> [0] ZiJW1g2nbQs9ekwK(at)pryzbyj2023
>
> But it doesn't have anything to do with "creating new partitions with
> parent's persistence". Maybe there was a merge conflict and the docs
> ended up in the wrong patch ?

Makes sense. Extracted this into a separate patch in v10.

> Also, defaults, storage options, compression are also copied. As will
> be anything else from LIKE. And since anything added in the future will
> also be copied, maybe it's better to just say that the tables will be
> created the same way as "LIKE .. INCLUDING ALL EXCLUDING ..", or
> similar. Otherwise, the next person who adds a new option for LIKE
> would have to remember to update this paragraph...

Reworded that way. Thank you.

> Also, extended stats objects are currently cloned to new child tables.
> But I suggested in [0] that they probably shouldn't be.

I will explore this. Do we copy extended stats when we do CREATE
TABLE ... PARTITION OF? I think we need to do the same here.

> > 007 – doc review by Justin [3]
>
> I suggest to drop this patch for now. I'll send some more minor fixes to
> docs and code comments once the other patches are settled.

Your edits are welcome. Dropped this for now. And waiting for the
next revision from you.

Links.
1. https://www.postgresql.org/message-id/CAPpHfduYuYECrqpHMgcOsNr%2B4j3uJK%2BJPUJ_zDBn-tqjjh3p1Q%40mail.gmail.com

------
Regards,
Alexander Korotkov
Supabase