De-dupicate Memoize cache keys

david-rowley · david-rowley · commit bc397e5cdb31 · 2024-01-26T20:51:36.000+13:00
It was possible when determining the cache keys for a Memoize path that if the same expr appeared twice in the parameterized path's ppi_clauses and/or in the Nested Loop's inner relation's lateral_vars. If this happened the Memoize node's cache keys would contain duplicates. This isn't a problem for correctness, all it means is that the cache lookups will be suboptimal due to having redundant work to do on every hash table lookup and insert. Here we adjust paraminfo_get_equal_hashops() to look for duplicates and ignore them when we find them. Author: David Rowley Reviewed-by: Richard Guo Discussion: https://postgr.es/m/422277.1706207562%40sss.pgh.pa.us
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
@@ -492,8 +492,16 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
 				return false;
 			}
 
-			*operators = lappend_oid(*operators, hasheqoperator);
-			*param_exprs = lappend(*param_exprs, expr);
+			/*
+			 * 'expr' may already exist as a parameter from a previous item in
+			 * ppi_clauses.  No need to include it again, however we'd better
+			 * ensure we do switch into binary mode if required.  See below.
+			 */
+			if (!list_member(*param_exprs, expr))
+			{
+				*operators = lappend_oid(*operators, hasheqoperator);
+				*param_exprs = lappend(*param_exprs, expr);
+			}
 
 			/*
 			 * When the join operator is not hashable then it's possible that
@@ -536,8 +544,16 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
 			return false;
 		}
 
-		*operators = lappend_oid(*operators, typentry->eq_opr);
-		*param_exprs = lappend(*param_exprs, expr);
+		/*
+		 * 'expr' may already exist as a parameter from the ppi_clauses.  No
+		 * need to include it again, however we'd better ensure we do switch
+		 * into binary mode.
+		 */
+		if (!list_member(*param_exprs, expr))
+		{
+			*operators = lappend_oid(*operators, typentry->eq_opr);
+			*param_exprs = lappend(*param_exprs, expr);
+		}
 
 		/*
 		 * We must go into binary mode as we don't have too much of an idea of
diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out
@@ -107,7 +107,7 @@ WHERE t1.unique1 < 10;', false);
          ->  Index Scan using tenk1_unique1 on tenk1 t1 (actual rows=10 loops=N)
                Index Cond: (unique1 < 10)
          ->  Memoize (actual rows=2 loops=N)
-               Cache Key: t1.two, t1.two
+               Cache Key: t1.two
                Cache Mode: binary
                Hits: 8  Misses: 2  Evictions: Zero  Overflows: 0  Memory Usage: NkB
                ->  Subquery Scan on t2 (actual rows=2 loops=N)
