From: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
---|---|
To: | Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru> |
Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |
Date: | 2025-07-29 09:38:22 |
Message-ID: | CAPpHfdsW5_rFLwTfkYuG42nX5RbJCVhaC2vkn8p5GsT8Lseuyw@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi, Dmitry!
I went through the patches. Both of them applied with a small conflict in
the parallel_schedule, which is easy to resolve.
+ ExecClearTuple(insertslot);
+
+ memcpy(insertslot->tts_values, srcslot->tts_values,
+ sizeof(Datum) * srcslot->tts_nvalid);
+ memcpy(insertslot->tts_isnull, srcslot->tts_isnull,
+ sizeof(bool) * srcslot->tts_nvalid);
+
+ ExecStoreVirtualTuple(insertslot);
Similar fragments are present in SplitPartitionMoveRows()
and MergePartitionsMoveRows(). Do you think we could use ExecCopySlot()
(or similar) instead?
The presence of the same name in the split partition list is checked with
equal() (similar concern was already raised about the merge case [1]). If
one of the names is schema-qualified, the error message is different.
Could we make them the same?
# alter table part_test split partition part_test_1_2 into (partition
part_test_1 for values in (1), partition part_test_1 for values in (2));
ERROR: name "part_test_1" is already used
LINE 1: ...artition part_test_1 for values in (1), partition part_test_...
^
Time: 1.194 ms
# alter table part_test split partition part_test_1_2 into (partition
part_test_1 for values in (1), partition public.part_test_1 for values in
(2));
ERROR: relation "part_test_1" already exists
Time: 6.187 ms
+ /* Search partition for current slot srcslot. */
+ foreach(listptr, partContexts)
+ {
+ pc = (SplitPartitionContext *) lfirst(listptr);
+
+ /* skip DEFAULT partition */
+ if (pc->partqualstate && ExecCheck(pc->partqualstate, econtext))
+ {
+ found = true;
+ break;
+ }
+ }
I see we're searching for a partition to place each row using the
sequential application of partition constraints. I concerned if this could
be exhausting when the number of new partitions is large. Could we use
something like binary search here?
New split/drop commands do the full reorganization of the involved
partitions. As Robert previously stated [2], there are other possible
strategies. While they are hard to implement, I don't think we need them
in the initial version. But I think it's worth mentioning in the docs that
we're completely rewriting the involved partitions. And this is not
recommended to use for merging very big partitions with small ones, and
splitting a small fraction of rows out of a very big partition.
Both patches could use pgindent run.
Links
1.
https://www.postgresql.org/message-id/CACJufxHHnJm6Jb2YQpuRU1RX__tO%3DJJNJ5%3DEUMuzif_KNxGd9A%40mail.gmail.com
2.
https://www.postgresql.org/message-id/CA%2BTgmoY0%3DbT_xBP8csR%3DMFE%3DFxGE2n2-me2-31jBOgEcLvW7ug%40mail.gmail.com
------
Regards,
Alexander Korotkov
Supabase
From | Date | Subject | |
---|---|---|---|
Next Message | Oleg Tselebrovskiy | 2025-07-29 09:45:25 | Re: ICU warnings during make installcheck and text_extensions test |
Previous Message | Jelte Fennema-Nio | 2025-07-29 09:35:36 | Re: Extension security improvement: Add support for extensions with an owned schema |