MIME-Version: 1.0
References: <CAMbWs48jLxn0pAPZpJ50EThZ569Xrw+=4Ac3QvkpQvNszbeoNg@mail.gmail.com>
 <CAMbWs48TGnh8s=9-hJge9Et4Crf9q+_oU4FVekLoX4ommi4CVg@mail.gmail.com>
 <CAMbWs4_E2HcTJ0b2srM3hwnQbH5SogkZMM399WUG4gTQSa1FOQ@mail.gmail.com>
 <CA+renyX=40YXhsfPTzn13oNOPO3TJ12CK9GX-2P2pvnQiScefA@mail.gmail.com> <CAApHDvqwyM_yy+OajtfHHu=E9qO_e_WxpriZRrcnnPGhULCW+g@mail.gmail.com>
In-Reply-To: <CAApHDvqwyM_yy+OajtfHHu=E9qO_e_WxpriZRrcnnPGhULCW+g@mail.gmail.com>
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 13 Jul 2023 17:29:42 +0800
Message-ID: <CAMbWs49dEHRPe8poM_K39r2uOsaOZcg+Y0B5a8tF7vW3uVR3mw@mail.gmail.com>
Subject: Re: Check lateral references within PHVs for memoize cache keys
To: David Rowley <dgrowleyml@gmail.com>
Cc: Paul A Jungwirth <pj@illuminatedcomputing.com>, 
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="00000000000084f53906005af9cf"
Archived-At: <https://www.postgresql.org/message-id/CAMbWs49dEHRPe8poM_K39r2uOsaOZcg%2BY0B5a8tF7vW3uVR3mw%40mail.gmail.com>
Precedence: bulk

--00000000000084f53906005af9cf
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sun, Jul 9, 2023 at 12:17=E2=80=AFPM David Rowley <dgrowleyml@gmail.com>=
 wrote:

> I just pushed a fix for this.  Thanks for reporting it.


BTW, I noticed a typo in the comment inside paraminfo_get_equal_hashops.

    foreach(lc, innerrel->lateral_vars)
    {
        Node       *expr =3D (Node *) lfirst(lc);
        TypeCacheEntry *typentry;

        /* Reject if there are any volatile functions in PHVs */
        if (contain_volatile_functions(expr))
        {
            list_free(*operators);
            list_free(*param_exprs);
            return false;
        }

The expressions in RelOptInfo.lateral_vars are not necessarily from
PHVs.  For instance

explain (costs off)
select * from t t1 join
    lateral (select * from t t2 where t1.a =3D t2.a offset 0) on true;
            QUERY PLAN
----------------------------------
 Nested Loop
   ->  Seq Scan on t t1
   ->  Memoize
         Cache Key: t1.a
         Cache Mode: binary
         ->  Seq Scan on t t2
               Filter: (t1.a =3D a)
(7 rows)

The lateral Var 't1.a' comes from the lateral subquery, not PHV.

This seems a typo from 63e4f13d.  How about we change it to the below?

-     /* Reject if there are any volatile functions in PHVs */
+     /* Reject if there are any volatile functions in lateral vars */

Thanks
Richard

--00000000000084f53906005af9cf
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><div class=3D"gmail_quote"><div=
 dir=3D"ltr" class=3D"gmail_attr">On Sun, Jul 9, 2023 at 12:17=E2=80=AFPM D=
avid Rowley &lt;<a href=3D"mailto:dgrowleyml@gmail.com">dgrowleyml@gmail.co=
m</a>&gt; wrote:</div><blockquote class=3D"gmail_quote" style=3D"margin:0px=
 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I just pushed a fix for this.=C2=A0 Thanks for reporting it.</blockquote><d=
iv><br></div><div>BTW, I noticed a typo in the comment inside paraminfo_get=
_equal_hashops.<br><br>=C2=A0 =C2=A0 foreach(lc, innerrel-&gt;lateral_vars)=
<br>=C2=A0 =C2=A0 {<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 Node =C2=A0 =C2=A0 =C2=
=A0 *expr =3D (Node *) lfirst(lc);<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 TypeCache=
Entry *typentry;<br><br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Reject if there are =
any volatile functions in PHVs */<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (contai=
n_volatile_functions(expr))<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 {<br>=C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 list_free(*operators);<br>=C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 list_free(*param_exprs);<br>=C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 return false;<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 }<br><br>=
The expressions in RelOptInfo.lateral_vars are not necessarily from<br>PHVs=
.=C2=A0 For instance<br><br>explain (costs off)<br>select * from t t1 join<=
br>=C2=A0 =C2=A0 lateral (select * from t t2 where t1.a =3D t2.a offset 0) =
on true;<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 QUERY PLAN<br>-------=
---------------------------<br>=C2=A0Nested Loop<br>=C2=A0 =C2=A0-&gt; =C2=
=A0Seq Scan on t t1<br>=C2=A0 =C2=A0-&gt; =C2=A0Memoize<br>=C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0Cache Key: t1.a<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Ca=
che Mode: binary<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0-&gt; =C2=A0Seq Scan =
on t t2<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Filter: (=
t1.a =3D a)<br>(7 rows)<br><br>The lateral Var &#39;t1.a&#39; comes from th=
e lateral subquery, not PHV.<br><br>This seems a typo from 63e4f13d.=C2=A0 =
How about we change it to the below?<br><br>- =C2=A0 =C2=A0 /* Reject if th=
ere are any volatile functions in PHVs */<br>+ =C2=A0 =C2=A0 /* Reject if t=
here are any volatile functions in lateral vars */<br><br>Thanks<br>Richard=
<br></div></div></div>

--00000000000084f53906005af9cf--