Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

From: Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru>
To: Zhihong Yu <zyu(at)yugabyte(dot)com>
Cc: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: 2022-06-01 18:58:42
Message-ID: 81aa663e-9785-5fc1-f711-10626cdeb365@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

1)
> For attachPartTable, the parameter wqueue is missing from comment.
> The parameters of CloneRowTriggersToPartition are called parent and partition.
> I think it is better to name the parameters to attachPartTable in a similar manner.
>
> For struct SplitPartContext, SplitPartitionContext would be better name.
>
> + /* Store partition contect into list. */
> contect -> context

Thanks, changed.

2)
> For transformPartitionCmdForMerge(), nested loop is used to detect duplicate names.
> If the number of partitions in partcmd->partlist, we should utilize map to speed up the check.

I'm not sure what we should utilize map in this case because chance that
number of merging partitions exceed dozens is low.
Is there a function example that uses a map for such a small number of
elements?

3)
> For check_parent_values_in_new_partitions():
>
> + if (!find_value_in_new_partitions(&key->partsupfunc[0],
> + key->partcollation, parts, nparts, datum, false))
> + found = false;
>
> It seems we can break out of the loop when found is false.

We have implicit "break" in "for" construction:

+ for (i = 0; i < boundinfo->ndatums && found; i++)

I'll change it to explicit "break;" to avoid confusion.

Attached patch with the changes described above.
--
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com

Attachment Content-Type Size
v3-0001-partitions-split-merge.patch text/plain 256.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhihong Yu 2022-06-01 19:10:22 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Previous Message Zhihong Yu 2022-06-01 17:58:20 Re: silence compiler warning in brin.c