From: Manuel Weitzman <manuelweitzman@gmail.com>
Message-Id: <F9C9FBE0-C535-49DA-B913-F11AC30F8F4E@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_3624CDBD-E171-4000-9B6E-C53FCA854A31"
Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.100.0.2.22\))
Subject: Re: Planning performance problem (67626.278ms)
Date: Tue, 29 Jun 2021 17:35:38 -0400
In-Reply-To: <500142.1624995808@sss.pgh.pa.us>
Cc: Ranier Vilela <ranier.vf@gmail.com>,
 David Rowley <dgrowleyml@gmail.com>,
 Jeremy Schneider <schnjere@amazon.com>,
 "pgsql-performance@lists.postgresql.org"
 <pgsql-performance@lists.postgresql.org>,
 Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
References: <6d58f68e-ed1a-c48e-1440-691554f6e1e7@9livesdata.com>
 <CAFj8pRDDnOVNcU7dx4+oKH0x4-DsmL3XRWppQ8Cf5wwJ=G2fPg@mail.gmail.com>
 <20190408144210.GG10080@telsasoft.com>
 <b145b428-3351-ec86-d522-100a5bf8ded7@amazon.com>
 <20190408232644.r6xixkeulsmomwfj@alap3.anarazel.de>
 <3f5e5673-06aa-bf7c-cdb7-0e053c45ba26@amazon.com>
 <CAApHDvq9o1J45is6w-3Gv1UgtuDQ4eQP4UhFPQSnQmEH_EijvQ@mail.gmail.com>
 <767433.1619015825@sss.pgh.pa.us>
 <EFE95962-F088-45E2-BD13-AA5012829A69@gmail.com>
 <F06D0C00-DE75-4688-98B0-85A46E8F312C@gmail.com>
 <CAEudQApVF7TX-EJXwBz4TUhH6NzNB-wXqhs4wj+qSe6Vw3H5-Q@mail.gmail.com>
 <1588548.1624234651@sss.pgh.pa.us>
 <39C2266C-3143-4A7A-BCAF-E0C8394F9D24@gmail.com>
 <500142.1624995808@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/F9C9FBE0-C535-49DA-B913-F11AC30F8F4E%40gmail.com>
Precedence: bulk


--Apple-Mail=_3624CDBD-E171-4000-9B6E-C53FCA854A31
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=us-ascii


> On 29-06-2021, at 15:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Manuel Weitzman <manuelweitzman@gmail.com> writes:
>>> On 20-06-2021, at 17:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> So ... the reason why there's not caching of get_actual_variable_range
>>> results already is that I'd supposed it wouldn't be necessary given
>>> the caching of selectivity estimates that happens at the RestrictInfo
>>> level.  I don't have any objection in principle to adding another
>>> caching layer if that one's not working well enough, but I think it'd
>>> be wise to first understand why it's needed.
> 
>> For what I could make out from the code, the caching done at the
>> RestrictInfo level is already saving a lot of work, but there's a
>> different RestrictInfo instance for each alternative path created by
>> make_one_rel().
> 
> That seems a bit broken; a given WHERE clause should produce only one
> RestrictInfo.  Can you provide a more concrete example?
> 

I added some logging to see hits and misses on cached_scansel() for
this query
> explain (analyze, buffers)
> select * from a
> join b b1 on (b1.a = a.a)
> join b b2 on (b2.a = a.a)
> where b1.a in (1,100,10000,1000000,1000001);

Apparently  there's a RestrictInfo for each possible way of doing merge
join (are those created dynamically for planning?), for example:
- a join (b1 join b2)
- b1 join (a join b2)
- b2 join (a join b1)

When the cost of a possible mergejoin path hasn't been computed yet,
then mergejoinscansel() would have to check the bloated index again.

I attached a patch so you can see the hits and misses on cached_scansel().
Each time there's a miss logged, there's also a different RestrictInfo
pointer involved.

Best regards,
Manuel

--Apple-Mail=_3624CDBD-E171-4000-9B6E-C53FCA854A31
Content-Disposition: attachment;
	filename=cached_scansel_hitmiss.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="cached_scansel_hitmiss.patch"
Content-Transfer-Encoding: 7bit

diff --git i/src/backend/optimizer/path/costsize.c w/src/backend/optimizer/path/costsize.c
index ffd2bf8783..180cdc5f10 100644
--- i/src/backend/optimizer/path/costsize.c
+++ w/src/backend/optimizer/path/costsize.c
@@ -3431,10 +3431,21 @@ cached_scansel(PlannerInfo *root, RestrictInfo *rinfo, PathKey *pathkey)
 		if (cache->opfamily == pathkey->pk_opfamily &&
 			cache->collation == pathkey->pk_eclass->ec_collation &&
 			cache->strategy == pathkey->pk_strategy &&
-			cache->nulls_first == pathkey->pk_nulls_first)
+			cache->nulls_first == pathkey->pk_nulls_first) {
+			elog(INFO, " [HIT][%p]\topfamily: %u, collation: %u, strategy: %d, nulls_first: %s, left_relids:%p, right_relids:%p",
+				 rinfo,
+				 pathkey->pk_opfamily, pathkey->pk_eclass->ec_collation,
+				 pathkey->pk_strategy, pathkey->pk_nulls_first ? "t" : "f",
+				 rinfo->left_relids, rinfo->right_relids);
 			return cache;
+		}
 	}
 
+	elog(INFO, "[MISS][%p]\topfamily:%u, collation: %u, strategy: %d, nulls_first: %s, left_relids:%p, right_relids:%p",
+		 rinfo,
+		 pathkey->pk_opfamily, pathkey->pk_eclass->ec_collation,
+		 pathkey->pk_strategy, pathkey->pk_nulls_first ? "t" : "f",
+		 rinfo->left_relids, rinfo->right_relids);
 	/* Nope, do the computation */
 	mergejoinscansel(root,
 					 (Node *) rinfo->clause,

--Apple-Mail=_3624CDBD-E171-4000-9B6E-C53FCA854A31--