On 12 December 2017 at 22:13, Amit Langote <langote_amit...@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
