Horton

Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Pavel Borisov <pashkin.elfe@gmail.com>

View thread
From: Pavel Borisov <pashkin.elfe@gmail.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, Dmitry Koval <d.koval@postgrespro.ru>, Alexander Lakhin <exclusion@gmail.com>, Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-04-26T13:33:33Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Adjust errcode in checkPartition()

  2. Fix usage of palloc() in MERGE/SPLIT PARTITION(s) code

  3. Implement ALTER TABLE ... SPLIT PARTITION ... command

  4. Implement ALTER TABLE ... MERGE PARTITIONS ... command

  5. Calculate agglevelsup correctly when Aggref contains a CTE.

  6. Use PqMsg_* macros in applyparallelworker.c.

  7. Restrict psql meta-commands in plain-text dumps.

  8. Ensure we have a snapshot when updating various system catalogs.

  9. Use specific collation where needed in new test

  10. Expand virtual generated columns in the planner

  11. Virtual generated columns

  12. Define PG_LOGICAL_DIR for path pg_logical/ in data folder

  13. Revert support for ALTER TABLE ... MERGE/SPLIT PARTITION(S) commands

  14. Avoid repeated table name lookups in createPartitionTable()

  15. Provide deterministic order for catalog queries in partition_split.sql

  16. Don't copy extended statistics during MERGE/SPLIT partition operations

  17. Fix the name collision detection in MERGE/SPLIT partition operations

  18. Fix regression tests conflict in 3ca43dbbb6

  19. Add permission check for MERGE/SPLIT partition operations

  20. Fix one more portability shortcoming in new test_pg_dump test.

  21. Inherit parent's AM for partition MERGE/SPLIT operations

  22. Add tab completion for partition MERGE/SPLIT operations

  23. Rename tables in tests of partition MERGE/SPLIT operations

  24. Make new partitions with parent's persistence during MERGE/SPLIT

  25. Document the way partition MERGE/SPLIT operations create new partitions

  26. Change the way ATExecMergePartitions() handles the name collision

  27. Grammar fixes for split/merge partitions code

  28. Checks for ALTER TABLE ... SPLIT/MERGE PARTITIONS ... commands

  29. Fix some grammer errors from error messages and codes comments

  30. Support TZ and OF format codes in to_timestamp().

  31. Support identity columns in partitioned tables

  32. Fix indentation in twophase.c

  33. Fix corner-case planner failure for MERGE.

  34. Doc: fix documentation example for bytea hex output format.

  35. Avoid repeated name lookups during table and index DDL.

Hi, Hackers!

On Thu, 25 Apr 2024 at 00:26, Justin Pryzby <pryzby@telsasoft.com> wrote:

> 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.
>
I've looked at the patchset:

0001 Look good.
0002 Also right with docs modification proposed by Justin.
0003:
Looks like unused code
5268             datum = cmpval ? list_nth(spec->lowerdatums, abs(cmpval) -
1) : NULL;
overridden by
5278                     datum = list_nth(spec->upperdatums, abs(cmpval) -
1);
and
5290                     datum = list_nth(spec->upperdatums, abs(cmpval) -
1);

Otherwise - good.

0004:
I suggest also getting rid of thee-noun compound words like:
salesperson_name. Maybe salesperson -> clerk? Or maybe use the same terms
like in pgbench: branches, tellers, accounts, balance.

0005: Good
0006: Patch is right
In comments:
+      New partitions will have the same table access method,
+      same column names and types as the partitioned table to which they
belong.
(I'd suggest to remove second "same")

Tests are passed. I suppose that it's better to add similar tests for
SPLIT/MERGE PARTITION(S)  to those covering ATTACH/DETACH PARTITION (e.g.:
subscription/t/013_partition.pl and regression tests)

Overall, great work! Thanks!

Regards,
Pavel Borisov,
Supabase.