On 2019-Oct-18, Etsuro Fujita wrote: > While reviewing the partitionwise-join patch, I noticed $Subject,ie, > this in create_list_bounds(): > > /* > * Never put a null into the values array, flag instead for > * the code further down below where we construct the actual > * relcache struct. > */ > if (null_index != -1) > elog(ERROR, "found null more than once"); > null_index = i; > > "the code further down below where we construct the actual relcache > struct" isn't in the same file anymore by refactoring by commit > b52b7dc25. How about modifying it like the attached?
Yeah, agreed. Instead of "the null comes from" I would use "the partition that stores nulls". While reviewing your patch I noticed a few places where we use an odd pattern in switches, which can be simplified as shown here. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 318d8ecae9..e051094d54 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -209,13 +209,9 @@ partition_bounds_create(PartitionBoundSpec **boundspecs, int nparts, return create_range_bounds(boundspecs, nparts, key, mapping); default: - elog(ERROR, "unexpected partition strategy: %d", - (int) key->strategy); - break; + Assert(false); + return NULL; /* keep compiler quiet */ } - - Assert(false); - return NULL; /* keep compiler quiet */ } /* @@ -1797,10 +1793,7 @@ qsort_partition_rbound_cmp(const void *a, const void *b, void *arg) static int get_partition_bound_num_indexes(PartitionBoundInfo bound) { - int num_indexes; - Assert(bound); - switch (bound->strategy) { case PARTITION_STRATEGY_HASH: @@ -1809,24 +1802,20 @@ get_partition_bound_num_indexes(PartitionBoundInfo bound) * The number of the entries in the indexes array is same as the * greatest modulus. */ - num_indexes = get_hash_partition_greatest_modulus(bound); - break; + return get_hash_partition_greatest_modulus(bound); case PARTITION_STRATEGY_LIST: - num_indexes = bound->ndatums; + return bound->ndatums; break; case PARTITION_STRATEGY_RANGE: /* Range partitioned table has an extra index. */ - num_indexes = bound->ndatums + 1; - break; + return bound->ndatums + 1; default: - elog(ERROR, "unexpected partition strategy: %d", - (int) bound->strategy); + Assert(false); + return 0; /* keep compiler quiet */ } - - return num_indexes; } /*