Horton

Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Junwang Zhao <zhjwpku@gmail.com>

View thread
From: Junwang Zhao <zhjwpku@gmail.com>
To: Dmitry Koval <d.koval@postgrespro.ru>
Cc: jian he <jian.universality@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2025-06-10T15:58:04Z
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 Dmitry,

On Tue, Jun 10, 2025 at 6:48 AM Dmitry Koval <d.koval@postgrespro.ru> wrote:
>
> Hi, Jian He!
>
> Thanks for the suggestions and patches!
> This email contains comments to three emails (05/06/2025).
> I hope to read two emails (for 06/06/2025) tomorrow.
>
> 1.
>  >What should we do when any to be merged partition has constraints?
>  >...
>  >Maybe this is expected, but we need to mention it somewhere and have
>  >some tests on it saying that MERGE PARTITIONS will effectively drop
>  >the partitions, so if any object depends on that partition
>  >then MERGE PARTITIONS can not be done.
>
> Added following phrases to the documentation (I hope this should be
> enough?):
>
> If merged partitions have individual constraints, those constraints will
> be dropped because command uses partitioned table as a model to create
> the constraints.
> If merged partitions have some objects dependent on them, the command
> can not be done (CASCADE is not used, an error will be returned).
>
>
> 2.
>  > ... so this error check can be performed as early as the
>  >transformPartitionCmdForMerge stage?
>
> Function createPartitionTable will be used for various other cases
> besides MERGE PARTITIONS: for SPLIT PARTITION, for PARTITION BY
> REFERENCE (I hope).
> So I think it's better to minimize the amount of code and not move the
> same one check into different functions (transformPartitionCmdForMerge,
> transformPartitionCmdForSplit, ...).
>
>
> 3.
>  >i think, we can do the following way:
>  >if (modelRel->rd_rel->relam)
>  >  elog(ERROR, "error");
>  >relamId = modelRel->rd_rel->relam;
>
> Can you clarify what is reason to change the current AM-logic for
> creating a new partition?
>
> +       /* Look up the access method for new relation. */
> +       relamId = (modelRel->rd_rel->relam != InvalidOid) ?
> modelRel->rd_rel->relam : HEAP_TABLE_AM_OID;
>
> (If AM is set for a partitioned table, then use it, otherwise use AM for
> heap tables.)
>
>
> 4.
>  > Attached is some refactoring in moveMergedTablesRows, hope it's
> straightforward.
>
> Thanks, these changes are useful.
>
>
> 5.
>  >bug in transformPartitionCmdForMerge "equal(name, name2))"
>  > ...
>  >ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022,
>  >public.sales_feb2022) INTO sales_feb_mar2022;
>  >ERROR:  lower bound of partition "sales_feb2022" conflicts with upper
>  >bound of previous partition "sales_feb2022"
>  >in this context. "sales_feb2022" is the same as "public.sales_feb2022".
>
> Added check and test for this case.
>
>
> 6.
>  >When using ALTER TABLE ... MERGE PARTITIONS, some of the new
>  >partition's properties will not be inherited from to be merged
>  >partitions; instead, they will be directly copied from the root
>  >partitioned table.
>  >So we need to test this behavior.
>  >The attached test file is for test table properties:
>  >(COMMENTS, COMPRESSION, DEFAULTS, GENERATED, STATISTICS, STORAGE).
>
> Some tests already exist (GENERATED, DEFAULTS) - see
> partition_merge.sql, lines after:
>
> +-- Test for:
> +--   * composite partition key;
> +--   * GENERATED column;
> +--   * column with DEFAULT value.
> ...
>
> But the complex test is probably also interesting.
> Test added.
>
> --
>
> Similar changes are made for the second commit (SPLIT PARTITION).
>
> --
> With best regards,
> Dmitry Koval
>
> Postgres Professional: http://postgrespro.com

I had one trivial comment and a question if you don't mind.

+ /* If existing rel is temp, it must belong to this session */
+ if (modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
+ !modelRel->rd_islocaltemp)
+ ereport(ERROR,
+ errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot create as partition of temporary relation of another
session"));

Would it be better to use RELATION_IS_OTHER_TEMP in this case?
I noticed that while other parts of tablecmds.c don’t use the macro,
all other files consistently use RELATION_IS_OTHER_TEMP.

+ /*
+ * We intended to create the partition with the same persistence as the
+ * parent table, but we still need to recheck because that might be
+ * affected by the search_path.  If the parent is permanent, so must be
+ * all of its partitions.
+ */

I have trouble understanding how this is possible, can you kindly
give me some guidance on this logic?

-- 
Regards
Junwang Zhao