Re: Hybrid Hash/Nested Loop joins and caching results from subplans
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: Andy Fan <zhihui.fan1213@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2020-12-08T07:15:52Z
Lists: pgsql-hackers
Attachments
- v12-0001-Allow-estimate_num_groups-to-pass-back-further-d.patch (text/plain) patch v12-0001
- v12-0004-Remove-code-duplication-in-nodeResultCache.c.patch (text/plain) patch v12-0004
- v12-0002-Allow-users-of-simplehash.h-to-perform-direct-de.patch (text/plain) patch v12-0002
- v12-0005-Use-a-Result-Cache-node-to-cache-results-from-su.patch (text/plain) patch v12-0005
- v12-0003-Add-Result-Cache-executor-node.patch (text/plain) patch v12-0003
I've attached another patchset that addresses some comments left by Zhihong Yu over on [1]. The version number got bumped to v12 instead of v11 as I still have a copy of the other version of the patch which I made some changes to and internally named v11. The patchset has grown 1 additional patch which is the 0004 patch. The review on the other thread mentioned that I should remove the code duplication for the full cache check that I had mostly duplicated between adding a new entry to the cache and adding tuple to an existing entry. I'm still a bit unsure that I like merging this into a helper function. One call needs the return value of the function to be a boolean value to know if it's still okay to use the cache. The other need the return value to be the cache entry. The patch makes the helper function return the entry and returns NULL to communicate the false value. I'm not a fan of the change and might drop it. The 0005 patch is now the only one that I think needs more work to make it good enough. This is Result Cache for subplans. I mentioned in [2] what my problem with that patch is. On Mon, 7 Dec 2020 at 12:50, David Rowley <dgrowleyml@gmail.com> wrote: > Basically, an extra argument in ExecBuildParamSetEqual() which allows > the TupleTableSlotOps for the left and right side to be set > individually. Previously I was passing a single TupleTableSlotOps of > TTSOpsMinimalTuple. The probeslot is a TTSOpsVirtual tuple, so > passing TTSOpsMinimalTuple causes the function to add a needless > EEOP_OUTER_FETCHSOME step to the expression. I also benchmarked that change and did see that it gives a small but notable improvement to the performance. David [1] https://www.postgresql.org/message-id/CALNJ-vRAgksPqjK-sAU+9gu3R44s_3jVPJ_5SDB++jjEkTntiA@mail.gmail.com [2] https://www.postgresql.org/message-id/CAApHDvpGX7RN+sh7Hn9HWZQKp53SjKaL=GtDzYheHWiEd-8moQ@mail.gmail.com
Commits
-
Add Result Cache executor node (take 2)
- 9eacee2e62d8 14.0 landed
-
Add Result Cache executor node
- b6002a796dc0 14.0 landed
-
Allow estimate_num_groups() to pass back further details about the estimation
- ed934d4fa30f 14.0 landed
-
Allow users of simplehash.h to perform direct deletions
- ff53d7b159b9 14.0 landed
-
Cache if PathTarget and RestrictInfos contain volatile functions
- f58b230ed0db 14.0 landed
-
Fix pull_varnos' miscomputation of relids set for a PlaceHolderVar.
- 55dc86eca70b 14.0 cited