Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Поиск
Список
Период
Сортировка
От Dmitry Koval
Тема Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Дата
Msg-id 81aa663e-9785-5fc1-f711-10626cdeb365@postgrespro.ru
обсуждение исходный текст
Ответ на Re: Add SPLIT PARTITION/MERGE PARTITIONS commands  (Zhihong Yu <zyu@yugabyte.com>)
Ответы Re: Add SPLIT PARTITION/MERGE PARTITIONS commands  (Zhihong Yu <zyu@yugabyte.com>)
Список 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
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Zhihong Yu
Дата:
Сообщение: Re: silence compiler warning in brin.c
Следующее
От: Zhihong Yu
Дата:
Сообщение: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands