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;
 }
 
 /*

Reply via email to