De-dupicate Memoize cache keys

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
index c0ba087b40a92e40c647506504c12b0e290da0dd..6aca66f1962bd62122203ed68b4665912c6fa5e7 100644 (file)
--- 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
index ca198ec3b800c71c7101c1573cd48260f8be1a8d..17bb3c8661dd3ba18574a29958925d4b56062263 100644 (file)
--- 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)