On Fri, 26 Jan 2024 at 16:51, Tom Lane <[email protected]> wrote:
> >> However ... it seems like we're not out of the woods yet. Why
> >> is Richard's proposed test case still showing
> >> + -> Memoize (actual rows=5000 loops=N)
> >> + Cache Key: t1.two, t1.two
> >> Seems like there is missing de-duplication logic, or something.
>
> > This seems separate and isn't quite causing the same problems as what
> > Richard wants to fix so I didn't touch this for now.
>
> Fair enough, but I think it might be worth pursuing later.
Here's a patch for that.
David
diff --git a/src/backend/optimizer/path/joinpath.c
b/src/backend/optimizer/path/joinpath.c
index c0ba087b40..8ecf9fe8dc 100644
--- 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 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 ca198ec3b8..17bb3c8661 100644
--- 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)
