MIME-Version: 1.0
References: <e711faa8-826d-434d-b197-ab51da518fe7@postgrespro.ru>
 <f79fa76f-de07-b930-0bc7-359d6cbab159@gmail.com> <b156efc2-5e6d-4eae-8e60-2d483987b42c@postgrespro.ru>
 <CAPpHfdv-uR663BO6G2kRuLEQjob+UBHuD3iyXF+qGHXQFRN0-Q@mail.gmail.com>
 <56e0000c-6b39-44ce-90b4-d3c3062e408c@postgrespro.ru> <CAPpHfdvShLh5txtQdh6=ZMCnzGZHv8W+RHAe8pY94rfzghWTAg@mail.gmail.com>
 <5dee3937-8e9f-cca4-11fb-737709a92b37@gmail.com> <f8b5cbf5-965e-4e5b-b506-33bbf41b0d50@postgrespro.ru>
 <CAPpHfdtLk5M4FV6C33DnBusWpsbwwmrQ83mpu7N8PZMupbftyw@mail.gmail.com>
 <CAPpHfdtB9L1C7HLE30WDQfQQgUZo5P+5=9kLr5vQSpDZ-2AV_A@mail.gmail.com> <ZilrByTp-pbz6Mvf@pryzbyj2023>
In-Reply-To: <ZilrByTp-pbz6Mvf@pryzbyj2023>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Sun, 28 Apr 2024 04:04:54 +0300
Message-ID: <CAPpHfds8syFKMRkOFVTwiqoQ1oqPs_bboPk+Sgt7cDKOZxnhhg@mail.gmail.com>
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Dmitry Koval <d.koval@postgrespro.ru>, Alexander Lakhin <exclusion@gmail.com>, 
	Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAPpHfds8syFKMRkOFVTwiqoQ1oqPs_bboPk%2BSgt7cDKOZxnhhg%40mail.gmail.com>
Precedence: bulk

Hi Justin,

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

On Wed, Apr 24, 2024 at 11:26=E2=80=AFPM Justin Pryzby <pryzby@telsasoft.co=
m> 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 m=
erging 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 dur=
ing 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@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 =E2=80=93 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