Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alexander Korotkov <aekorotkov@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, Dmitry Koval <d.koval@postgrespro.ru>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, pgsql-hackers@lists.postgresql.org
Date: 2024-04-19T11:34:46Z
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.

On Thu, Apr 11, 2024 at 10:20:53PM -0400, Robert Haas wrote:
> On Thu, Apr 11, 2024 at 9:54 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > I think we shouldn't unconditionally copy schema name and
> > relpersistence from the parent table.  Instead we should throw the
> > error on a mismatch like CREATE TABLE ... PARTITION OF ... does.  I'm
> > working on revising this fix.
> 
> We definitely shouldn't copy the schema name from the parent table. It
> should be possible to schema-qualify the new partition names, and if
> you don't, then the search_path should determine where they get
> placed.

+1.  Alexander Lakhin reported an issue with schemas and SPLIT, and I
noticed an issue with schemas with MERGE.  The issue I hit is occurs
when MERGE'ing into a partition with the same name, and it's fixed like
so:

--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21526,8 +21526,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	{
 		/* Create partition table with generated temporary name. */
 		sprintf(tmpRelName, "merge-%u-%X-tmp", RelationGetRelid(rel), MyProcPid);
-		mergePartName = makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
-									 tmpRelName, -1);
+		mergePartName = makeRangeVar(mergePartName->schemaname, tmpRelName, -1);
 	}
 	createPartitionTable(mergePartName,
 						 makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),

> One of the things I dislike about this type of feature -- not this
> implementation specifically, but just this kind of idea in general --
> is that the syntax mentions a whole bunch of tables but in a way where
> you can't set their properties. Persistence, reloptions, whatever.
> There's just no place to mention any of that stuff - and if you wanted
> to create a place, you'd have to invent special syntax for each
> separate thing. That's why I think it's good that the normal way of
> creating a partition is CREATE TABLE .. PARTITION OF. Because that
> way, we know that the full power of the CREATE TABLE statement is
> always available, and you can set anything that you could set for a
> table that is not a partition.

Right.  The current feature is useful and will probably work for 90% of
people's partitioned tables.

Currently, CREATE TABLE .. PARTITION OF does not create stats objects on
the child table, but MERGE PARTITIONS does, which seems strange.
Maybe stats should not be included on the new child ?

Note that stats on parent table are not analagous to indexes -
partitioned indexes do nothing other than cause indexes to be created on
any new/attached partitions.  But stats objects on the parent 1) cause
extended stats to be collected and computed across the whole partition
heirarchy, and 2) do not cause stats to be computed for the individual
partitions.

Partitions can have different column definitions, for example null
constraints, FKs, defaults.  And currently, if you MERGE partitions,
those will all be lost (or rather, replaced by whatever LIKE parent
gives).  I think that's totally fine - anyone using different defaults
on child tables could either not use MERGE PARTITIONS, or fix up the
defaults afterwards.  There's not much confusion that the details of the
differences between individual partitions will be lost when the
individual partitions are merged and no longer exist.
But I think it'd be useful to document how the new partitions will be
constructed.

-- 
Justin