Re: Add SPLIT PARTITION/MERGE PARTITIONS commands - Mailing list pgsql-hackers

On Mon, Apr 22, 2024 at 01:31:48PM +0300, Alexander Korotkov wrote:
> Hi!
> 
> On Fri, Apr 19, 2024 at 4:29 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Fri, Apr 19, 2024 at 2:26 AM Dmitry Koval <d.koval@postgrespro.ru> wrote:
> > > 18.04.2024 19:00, Alexander Lakhin wrote:
> > > > leaves a strange constraint:
> > > > \d+ t*
> > > >                                            Table "public.tp_0"
> > > > ...
> > > > Not-null constraints:
> > > >      "merge-16385-26BCB0-tmp_i_not_null" NOT NULL "i"
> > >
> > > Thanks!
> > > Attached fix (with test) for this case.
> > > The patch should be applied after patches
> > > v6-0001- ... .patch ... v6-0004- ... .patch
> >
> > I've incorporated this fix with 0001 patch.
> >
> > Also added to the patchset
> > 005 – tab completion by Dagfinn [1]
> > 006 – draft fix for table AM issue spotted by Alexander Lakhin [2]
> > 007 – doc review by Justin [3]
> >
> > I'm continuing work on this.
> >
> > Links
> > 1. https://www.postgresql.org/message-id/87plumiox2.fsf%40wibble.ilmari.org
> > 2. https://www.postgresql.org/message-id/84ada05b-be5c-473e-6d1c-ebe5dd21b190%40gmail.com
> > 3. https://www.postgresql.org/message-id/ZiGH0xc1lxJ71ZfB%40pryzbyj2023
> 
> 0001
> The way we handle name collisions during MERGE PARTITIONS operation is
> reworked by integration of patch [3].  This makes note about commit in
> [2] not relevant.

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)

> 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@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 ?

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...

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

> 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.

-- 
Justin