Re: Hybrid Hash/Nested Loop joins and caching results from subplans
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>,
Konstantin Knizhnik <k.knizhnik@postgrespro.ru>, Andy Fan <zhihui.fan1213@gmail.com>,
Alvaro Herrera <alvherre@alvh.no-ip.org>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-12T01:49:29Z
Lists: pgsql-hackers
Attachments
- v15-0001-Cache-PathTarget-and-RestrictInfo-s-volatility.patch (text/plain) patch v15-0001
- resultcache_v14_vs_v15.png (image/png)
- v15-0002-Allow-estimate_num_groups-to-pass-back-further-d.patch (text/plain) patch v15-0002
- v15-0003-Allow-users-of-simplehash.h-to-perform-direct-de.patch (text/plain) patch v15-0003
- v15-0004-Add-Result-Cache-executor-node.patch (text/plain) patch v15-0004
- v15-0005-Remove-code-duplication-in-nodeResultCache.c.patch (text/plain) patch v15-0005
On Tue, 23 Feb 2021 at 18:43, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I doubt it's that bad. We could cache such info in RestrictInfo > for quals, or PathTarget for tlists, without much new notational > overhead. That doesn't cover everything the planner deals with > of course, but it would cover enough that you'd be chasing pretty > small returns to worry about more. This seems like a pretty good idea. So I coded it up. The 0001 patch adds a has_volatile bool field to RestrictInfo and sets it when building the RestrictInfo. I've also added has_volatile_expr to PathTarget which is maintained when first building, then adding new Exprs to the PathTarget. I've modified a series of existing calls to contain_volatile_functions() to check these new fields first. This seems pretty good even without the Result Cache patch as it saves a few duplicate checks for volatile functions. For example, both check_hashjoinable() and check_mergejoinable() call contain_volatile_functions(). Now they just check the has_volatile flag after just calling contain_volatile_functions() once per RestrictInfo when the RestrictInfo is built. I tested the performance of just 0001 against master and I did see the overall planning and execution time of the join order benchmark query 29b go from taking 104.8 ms down to 103.7 ms. For the Result Cache patch, I've coded it to make use of these new fields instead of calling contain_volatile_functions(). I also noticed that I can use the pre-cached RestrictInfo->hashjoinoperator field when it's set. This will be the same operator as we'd be looking up using lookup_type_cache() anyway. With Result Cache we can also cache the tuples from non-equality joins, e.g ON t1.x > t2.y, but we still need to look for the hash equality operator in that case. I had thoughts that it might be worth adding an additional field to RestrictInfo for resultcacheoperator to save having to look it up each time for when hashjoinoperator is not set. We must still call estimate_num_groups() once each time we create a ResultCachePath. That's required in order to estimate the cache hits. All other join operators only care about clauselist_selectivity(). The selectivity estimates for those are likely to be cached in the RestictInfo to save having to do it again next time. There's no caching for estimate_num_groups(). I don't quite see any way to add caching for this, however. I've attached the updated patches. It took v14 144.6 ms to plan and execute query 29b. It takes v15 128.5 ms. Master takes 104.8 ms (see attached graph). The caching has improved the planning performance quite a bit. Thank you for the suggestion. David
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