Minor cleanup of partbounds.c

Started by Etsuro Fujitaover 5 years ago3 messages
#1Etsuro Fujita
etsuro.fujita@gmail.com
1 attachment(s)

Here is a patch for minor cleanup of the partbounds.c changes made by
commit c8434d64c: 1) removes a useless assignment (in normal builds)
and 2) improves comments a little.

Best regards,
Etsuro Fujita

Attachments:

partbounds-cleanup.patchapplication/octet-stream; name=partbounds-cleanup.patchDownload
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 419c8fe845..58f9b46289 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -1020,8 +1020,6 @@ partition_bounds_merge(int partnatts,
 					   JoinType jointype,
 					   List **outer_parts, List **inner_parts)
 {
-	PartitionBoundInfo outer_binfo = outer_rel->boundinfo;
-
 	/*
 	 * Currently, this function is called only from try_partitionwise_join(),
 	 * so the join type should be INNER, LEFT, FULL, SEMI, or ANTI.
@@ -1031,10 +1029,10 @@ partition_bounds_merge(int partnatts,
 		   jointype == JOIN_ANTI);
 
 	/* The partitioning strategies should be the same. */
-	Assert(outer_binfo->strategy == inner_rel->boundinfo->strategy);
+	Assert(outer_rel->boundinfo->strategy == inner_rel->boundinfo->strategy);
 
 	*outer_parts = *inner_parts = NIL;
-	switch (outer_binfo->strategy)
+	switch (outer_rel->boundinfo->strategy)
 	{
 		case PARTITION_STRATEGY_HASH:
 
@@ -1075,7 +1073,7 @@ partition_bounds_merge(int partnatts,
 
 		default:
 			elog(ERROR, "unexpected partition strategy: %d",
-				 (int) outer_binfo->strategy);
+				 (int) outer_rel->boundinfo->strategy);
 			return NULL;		/* keep compiler quiet */
 	}
 }
@@ -1528,7 +1526,7 @@ merge_range_bounds(int partnatts, FmgrInfo *partsupfuncs,
 													 &next_index);
 			Assert(merged_index >= 0);
 
-			/* Get the range of the merged partition. */
+			/* Get the range bounds of the merged partition. */
 			get_merged_range_bounds(partnatts, partsupfuncs,
 									partcollations, jointype,
 									&outer_lb, &outer_ub,
@@ -1833,7 +1831,7 @@ merge_matching_partitions(PartitionMap *outer_map, PartitionMap *inner_map,
 
 	/*
 	 * If neither of them has been merged, merge them.  Otherwise, if one has
-	 * been merged with a dummy relation on the other side (and the other
+	 * been merged with a dummy partition on the other side (and the other
 	 * hasn't yet been merged with anything), re-merge them.  Otherwise, they
 	 * can't be merged, so return -1.
 	 */
#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Etsuro Fujita (#1)
Re: Minor cleanup of partbounds.c

On 2020-Sep-09, Etsuro Fujita wrote:

Here is a patch for minor cleanup of the partbounds.c changes made by
commit c8434d64c: 1) removes a useless assignment (in normal builds)

LGTM.

and 2) improves comments a little.

No objection to changing "bounds" to "range bounds".

I think the point other is to replace the only appearance of "dummy
relation" to better match the extensive use of "dummy partition" in this
file. The concept of a "dummy relation" is well established in the
planner. I didn't know if "dummy partition" is itself a concept
(apparently in the newfangled partition-wise join stuff), or just
glorified wording to say "a dummy relation that happens to be a
partition". Looking at is_dummy_partition, apparently a dummy partition
is either a dummy relation or a partition that doesn't have a
RelOptInfo. So my conclusion is that this wording is okay to change
too.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#3Etsuro Fujita
etsuro.fujita@gmail.com
In reply to: Alvaro Herrera (#2)
Re: Minor cleanup of partbounds.c

On Thu, Sep 10, 2020 at 2:05 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2020-Sep-09, Etsuro Fujita wrote:

Here is a patch for minor cleanup of the partbounds.c changes made by
commit c8434d64c: 1) removes a useless assignment (in normal builds)

LGTM.

and 2) improves comments a little.

No objection to changing "bounds" to "range bounds".

I think the point other is to replace the only appearance of "dummy
relation" to better match the extensive use of "dummy partition" in this
file. The concept of a "dummy relation" is well established in the
planner. I didn't know if "dummy partition" is itself a concept
(apparently in the newfangled partition-wise join stuff), or just
glorified wording to say "a dummy relation that happens to be a
partition". Looking at is_dummy_partition, apparently a dummy partition
is either a dummy relation or a partition that doesn't have a
RelOptInfo. So my conclusion is that this wording is okay to change
too.

Cool!

I pushed the patch. Thanks for reviewing!

Best regards,
Etsuro Fujita