From d69d01ba9d8d774487032459ebb83d2086715f01 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 5 Dec 2022 12:36:40 -0500 Subject: [PATCH] Fix Memoize to work with partitionwise joining. A couple of places weren't up to speed for this. By sheer good luck, we didn't fail but just selected a non-memoized join plan, at least in the test case we have. Nonetheless, it's a bug, and I'm not quite sure that it couldn't have worse consequences in other examples. So back-patch to v14 where Memoize came in. Richard Guo Discussion: https://postgr.es/m/CAMbWs48GkNom272sfp0-WeD6_0HSR19BJ4H1c9ZKSfbVnJsvRg@mail.gmail.com --- src/backend/optimizer/path/joinpath.c | 3 ++- src/backend/optimizer/util/pathnode.c | 1 + src/include/nodes/pathnodes.h | 4 +-- src/test/regress/expected/memoize.out | 39 +++++++++++++++++++++++++++ src/test/regress/sql/memoize.sql | 19 +++++++++++++ 5 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 2a3f0ab7bf..4d09881259 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -597,7 +597,8 @@ get_memoize_path(PlannerInfo *root, RelOptInfo *innerrel, /* Check if we have hash ops for each parameter to the path */ if (paraminfo_get_equal_hashops(root, inner_path->param_info, - outerrel, + outerrel->top_parent ? + outerrel->top_parent : outerrel, innerrel, ¶m_exprs, &hash_operators, diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index 5379c087a1..55deee555a 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -4246,6 +4246,7 @@ do { \ FLAT_COPY_PATH(mpath, path, MemoizePath); REPARAMETERIZE_CHILD_PATH(mpath->subpath); + ADJUST_CHILD_ATTRS(mpath->param_exprs); new_path = (Path *) mpath; } break; diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index dbaa9bb54d..12624e6adb 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -1865,8 +1865,8 @@ typedef struct MemoizePath { Path path; Path *subpath; /* outerpath to cache tuples from */ - List *hash_operators; /* hash operators for each key */ - List *param_exprs; /* cache keys */ + List *hash_operators; /* OIDs of hash equality ops for cache keys */ + List *param_exprs; /* expressions that are cache keys */ bool singlerow; /* true if the cache entry is to be marked as * complete after caching the first record. */ bool binary_mode; /* true when cache key should be compared bit diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out index 00438eb1ea..de43afa76e 100644 --- a/src/test/regress/expected/memoize.out +++ b/src/test/regress/expected/memoize.out @@ -197,6 +197,45 @@ SELECT * FROM strtest s1 INNER JOIN strtest s2 ON s1.t >= s2.t;', false); (8 rows) DROP TABLE strtest; +-- Ensure memoize works with partitionwise join +SET enable_partitionwise_join TO on; +CREATE TABLE prt (a int) PARTITION BY RANGE(a); +CREATE TABLE prt_p1 PARTITION OF prt FOR VALUES FROM (0) TO (10); +CREATE TABLE prt_p2 PARTITION OF prt FOR VALUES FROM (10) TO (20); +INSERT INTO prt VALUES (0), (0), (0), (0); +INSERT INTO prt VALUES (10), (10), (10), (10); +CREATE INDEX iprt_p1_a ON prt_p1 (a); +CREATE INDEX iprt_p2_a ON prt_p2 (a); +ANALYZE prt; +SELECT explain_memoize(' +SELECT * FROM prt t1 INNER JOIN prt t2 ON t1.a = t2.a;', false); + explain_memoize +------------------------------------------------------------------------------------------ + Append (actual rows=32 loops=N) + -> Nested Loop (actual rows=16 loops=N) + -> Index Only Scan using iprt_p1_a on prt_p1 t1_1 (actual rows=4 loops=N) + Heap Fetches: N + -> Memoize (actual rows=4 loops=N) + Cache Key: t1_1.a + Cache Mode: logical + Hits: 3 Misses: 1 Evictions: Zero Overflows: 0 Memory Usage: NkB + -> Index Only Scan using iprt_p1_a on prt_p1 t2_1 (actual rows=4 loops=N) + Index Cond: (a = t1_1.a) + Heap Fetches: N + -> Nested Loop (actual rows=16 loops=N) + -> Index Only Scan using iprt_p2_a on prt_p2 t1_2 (actual rows=4 loops=N) + Heap Fetches: N + -> Memoize (actual rows=4 loops=N) + Cache Key: t1_2.a + Cache Mode: logical + Hits: 3 Misses: 1 Evictions: Zero Overflows: 0 Memory Usage: NkB + -> Index Only Scan using iprt_p2_a on prt_p2 t2_2 (actual rows=4 loops=N) + Index Cond: (a = t1_2.a) + Heap Fetches: N +(21 rows) + +DROP TABLE prt; +RESET enable_partitionwise_join; -- Exercise Memoize code that flushes the cache when a parameter changes which -- is not part of the cache key. -- Ensure we get a Memoize plan diff --git a/src/test/regress/sql/memoize.sql b/src/test/regress/sql/memoize.sql index 0979bcdf76..17c5b4bfab 100644 --- a/src/test/regress/sql/memoize.sql +++ b/src/test/regress/sql/memoize.sql @@ -104,6 +104,25 @@ SELECT * FROM strtest s1 INNER JOIN strtest s2 ON s1.t >= s2.t;', false); DROP TABLE strtest; +-- Ensure memoize works with partitionwise join +SET enable_partitionwise_join TO on; + +CREATE TABLE prt (a int) PARTITION BY RANGE(a); +CREATE TABLE prt_p1 PARTITION OF prt FOR VALUES FROM (0) TO (10); +CREATE TABLE prt_p2 PARTITION OF prt FOR VALUES FROM (10) TO (20); +INSERT INTO prt VALUES (0), (0), (0), (0); +INSERT INTO prt VALUES (10), (10), (10), (10); +CREATE INDEX iprt_p1_a ON prt_p1 (a); +CREATE INDEX iprt_p2_a ON prt_p2 (a); +ANALYZE prt; + +SELECT explain_memoize(' +SELECT * FROM prt t1 INNER JOIN prt t2 ON t1.a = t2.a;', false); + +DROP TABLE prt; + +RESET enable_partitionwise_join; + -- Exercise Memoize code that flushes the cache when a parameter changes which -- is not part of the cache key.