Message-ID: <2a76386f-a104-461b-8d15-ff6c4d762925@gmail.com>
Date: Fri, 12 Jul 2024 10:18:10 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Check lateral references within PHVs for memoize cache keys
To: Richard Guo <guofenglinux@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>,
 David Rowley <dgrowleyml@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
References: <CAMbWs48jLxn0pAPZpJ50EThZ569Xrw+=4Ac3QvkpQvNszbeoNg@mail.gmail.com>
 <CAMbWs48TGnh8s=9-hJge9Et4Crf9q+_oU4FVekLoX4ommi4CVg@mail.gmail.com>
 <CAMbWs4_E2HcTJ0b2srM3hwnQbH5SogkZMM399WUG4gTQSa1FOQ@mail.gmail.com>
 <1724688.1688837323@sss.pgh.pa.us>
 <CAMbWs49+Cjoy0S0xkCRDcHXGHvsYLOdvr9jq9OTONOBnsgzXOg@mail.gmail.com>
 <CAMbWs4_imG5C8rXt7xdU7zf6whUDc2rdDun+Vtrowcmxb41CzA@mail.gmail.com>
 <CAMbWs49Z6TrH4qJuudPx2toeJutPOV1CyvbL3GE1QNg_LaATEg@mail.gmail.com>
 <CAMbWs48A3Zd2=wZ9xCM07qHEeGBw-X2nodj2n_qnHNtBqiA73g@mail.gmail.com>
 <CAMbWs4-n-bc=cxXwn7oY0E8YojSwiVLUpz6+OqPre3_YJs1_NQ@mail.gmail.com>
 <a19ac195-0dd9-43f1-94a8-8ab39e48dab7@gmail.com>
 <CAMbWs4_CQR9ikyAhjH0CwzCdp+FcRAeoVsi5Oohd7s=c5RZABg@mail.gmail.com>
Content-Language: en-AU
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: <CAMbWs4_CQR9ikyAhjH0CwzCdp+FcRAeoVsi5Oohd7s=c5RZABg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/2a76386f-a104-461b-8d15-ff6c4d762925%40gmail.com>
Precedence: bulk

On 7/11/24 16:18, Richard Guo wrote:
> On Fri, Jun 28, 2024 at 10:14 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
>> I got the point about Memoize over join, but as a join still calls
>> replace_nestloop_params to replace parameters in its clauses, why not to
>> invent something similar to find Memoize keys inside specific JoinPath
>> node? It is not the issue of this patch, though - but is it doable?
> 
> I don't think it's impossible to do, but I'm skeptical that there's an
> easy way to identify all the cache keys for joinrels, without having
> available ppi_clauses and lateral_vars.
Ok

> 
>> IMO, the code:
>> if (bms_nonempty_difference(outerPlan->chgParam, node->keyparamids))
>>     cache_purge_all(node);
>>
>> is a good place to check an assertion: is it really the parent query
>> parameters that make a difference between memoize keys and node list of
>> parameters?
> 
> I don't think we have enough info available here to identify which
> params within outerPlan->chgParam are from outer levels.  Maybe we can
> store root->outer_params in the MemoizeState node to help with this
> assertion, but I'm not sure if it's worth the trouble.
Got it
> 
> Attached is an updated version of this patch.
I'm not sure about stability of output format of AVG aggregate across 
different platforms. Maybe better to return the result of comparison 
between the AVG() and expected value?

-- 
regards, Andrei Lepikhov