Code review for dynahash change.

macdice · macdice · commit ff28809feb44 · 2020-09-19T15:53:06.000+12:00
Commit be0a666 left behind a comment about the order of some tests that didn't make sense without the expensive division, and in fact we might as well change the order to one that fails more cheaply most of the time as a micro-optimization. Also, remove the "+ 1" applied to max_bucket, to drop an instruction and match the original behavior. Per review from Tom Lane. Discussion: https://postgr.es/m/VI1PR0701MB696044FC35013A96FECC7AC8F62D0%40VI1PR0701MB6960.eurprd07.prod.outlook.com
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
@@ -963,11 +963,10 @@ hash_search_with_hash_value(HTAB *hashp,
 	{
 		/*
 		 * Can't split if running in partitioned mode, nor if frozen, nor if
-		 * table is the subject of any active hash_seq_search scans.  Strange
-		 * order of these tests is to try to check cheaper conditions first.
+		 * table is the subject of any active hash_seq_search scans.
 		 */
-		if (!IS_PARTITIONED(hctl) && !hashp->frozen &&
-			hctl->freeList[0].nentries > (long) (hctl->max_bucket + 1) &&
+		if (hctl->freeList[0].nentries > (long) hctl->max_bucket &&
+			!IS_PARTITIONED(hctl) && !hashp->frozen &&
 			!has_seq_scans(hashp))
 			(void) expand_table(hashp);
 	}
