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
