Horton

Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>, Dmitry Koval <d.koval@postgrespro.ru>, Noah Misch <noah@leadboat.com>, pgsql-hackers@lists.postgresql.org
Date: 2025-01-31T09:19:29Z
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!

I'd like to share some thoughts on which particular way this patch could go.

On Thu, Aug 22, 2024 at 8:25 PM Robert Haas <robertmhaas@gmail.com> wrote:
> Here, aside from the name lookup issues, there are also problems with
> expression evaluation: we can't split partitions without reindexing
> rows that those partitions contain, and it is critical to think
> through which is going to do the evaluation and make sure it's
> properly sandboxed. I think we might need
> SECURITY_RESTRICTED_OPERATION here.

+1 for use SECURITY_RESTRICTED_OPERATION

> Another thing I want to highlight if you do have another go at this
> patch is that it's really critical to think about where every single
> property of the newly-created tables comes from. The original patch
> didn't consider relpersistence or tableam, and here I just discovered
> that owner is also an issue that probably needs more consideration,
> but it goes way beyond that. For example, I was surprised to discover
> that if I put per-partition constraints or triggers on a partition and
> then split it, they were not duplicated to the new partitions. Now,
> maybe that's actually the behavior we want -- I'm not 100% positive --
> but it sure wasn't what I was expecting. If we did duplicate them when
> splitting, then what's supposed to happen when merging occurs? That is
> not at all obvious, at least to me, but it needs careful thought. ACLs
> and rules and default values and foreign keys (both outbond and
> inbound) all need to be considered too, along with 27 other things
> that I'm sure I'm not thinking about right now. Some of this behavior
> should probably be explicitly documented, but all of it should be
> considered carefully enough before commit to avoid surprises later. I
> say that both from a security point of view and also just from a user
> experience point of view. Even if things aren't insecure, they can
> still be annoying, but it's not uncommon in cases like this for
> annoying things to turn out to also be insecure.

Yes, I think it's a good idea to duplicate dependent objects of split
partition to new partitions.  But it important to very carefully check
user have relevant permissions for all of them.  We could also provide
a syntax to exclude some of them (and even define new ones?), but I
strongly suspect that would overcomplicate patch for now and we need
to postpone this.

Regarding the merge, I think it would be good to provide a syntax to
let user choose a model partition between partitions to be merged.

> Finally, if you do revisit this, I believe it would be a good idea to
> think a bit harder about how data is moved around. My impression (and
> please correct me if I am mistaken) is that currently, any split or
> merge operation rewrites all the data in the source partition(s). If a
> large partition is being split nearly equally, I think that has a good
> chance of being optimal, but I think that might be the only case. If
> we're merging partitions, wouldn't it be better to adjust the
> constraints on the first partition -- or perhaps the largest partition
> if we want to be clever -- and insert the data from all of the others
> into it? Maybe that would even have syntax that puts the user in
> control of which partition survives, e.g. ALTER TABLE tab1 MERGE
> PARTITION part1 WITH part2, part3, .... That would also make it really
> obvious to the user what all of the properties of part1 will be after
> the merge: they will be exactly the same as they were before the
> merge, except that the partition constraint will have been adjusted.
> You basically dodge everything in the previous paragraph in one shot,
> and it seems like it would also be faster. Splitting there's no
> similar get-out-of-jail free card, at least not that I can see. Even
> if you add syntax that splits a partition by using INSERT/DELETE to
> move some rows to a newly-created partition, you still have to make at
> least one new partition. But possibly that syntax is worth having
> anyway, because it would be a lot quicker in the case of a highly
> asymmetric split. On the other hand, maybe even splits are much more
> likely and we don't really need it. I don't know.

Hmm... I think the important aspect for this DDL operation is to be
atomic and transactional.  And that seems to be extremely hard to
achieve if we move the data between existing relnodes.  How can we
rollback or recover after error?  So, it least for initial
implementation I would leave data movement as it is.

------
Regards,
Alexander Korotkov
Supabase