On 12 December 2017 at 22:13, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Attached updated patches.
Hi Amit,
I was also wondering about your thoughts on the design of
get_partitions_for_keys() and more generally how there are many
functions which have some special treatment doing something based on
->strategy == PARTITION_STRATEGY_XXX.
If I do:
git grep PARTITION_STRATEGY -- src/backend/catalog/partition.c | wc -l
I get 62 matches, most of which are case statements, and most of the
remainder are things like if (key->strategy ==
PARTITION_STRATEGY_HASH).
git grep --show-function PARTITION_STRATEGY -- src/backend/catalog/partition.c
shows that get_partitions_for_keys() is probably the most guilty of
having the most strategy condition tests.
Also, if we look at get_partitions_for_keys() there's an unconditional:
memset(hash_isnull, false, sizeof(hash_isnull));
which is only used for PARTITION_STRATEGY_HASH, but LIST and RANGE
must pay the price of that memset. Perhaps it's not expensive enough
to warrant only doing that when partkey->strategy ==
PARTITION_STRATEGY_HASH, but it does make me question if we should
have 3 separate functions for this and just have a case statement to
call the correct one.
I think if we were to put this off as something we'll fix later, then
the job would just become harder and harder as time goes on.
It might have been fine when we just had RANGE and LIST partitioning,
but I think HASH really tips the scales over to this being needed.
What do you think?
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
