Horton

Thread

Commits

  1. Don't Memoize lateral joins with volatile join conditions

  1. Check volatile functions in ppi_clauses for memoize node

    Richard Guo <guofenglinux@gmail.com> — 2023-08-04T10:26:21Z

    In get_memoize_path() we have a theory about avoiding memoize node if
there are volatile functions in the inner rel's target/restrict list.

 /*
  * We can't use a memoize node if there are volatile functions in the
  * inner rel's target list or restrict list.  A cache hit could reduce the
  * number of calls to these functions.
  */
 if (contain_volatile_functions((Node *) innerrel->reltarget))
     return NULL;

 foreach(lc, innerrel->baserestrictinfo)
 {
     RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);

     if (contain_volatile_functions((Node *) rinfo))
         return NULL;
 }

It seems that the check for restrict list is not thorough, because for a
parameterized scan we are supposed to also consider all the join clauses
available from the outer rels, ie, ppi_clauses.  For instance,

create table t (a float);
insert into t values (1.0), (1.0), (1.0), (1.0);
analyze t;

explain (costs off)
select * from t t1 left join lateral
    (select t1.a as t1a, t2.a as t2a from t t2) s
on t1.a = s.t2a + random();
                  QUERY PLAN
-----------------------------------------------
 Nested Loop Left Join
   ->  Seq Scan on t t1
   ->  Memoize
         Cache Key: t1.a
         Cache Mode: binary
         ->  Seq Scan on t t2
               Filter: (t1.a = (a + random()))
(7 rows)

According to the theory we should not use memoize node for this query
because of the volatile function in the inner side.  So propose a patch
to fix that.

Thanks
Richard

  2. Re: Check volatile functions in ppi_clauses for memoize node

    David Rowley <dgrowleyml@gmail.com> — 2023-08-07T10:19:08Z

    On Fri, 4 Aug 2023 at 22:26, Richard Guo <guofenglinux@gmail.com> wrote:
> explain (costs off)
> select * from t t1 left join lateral
>     (select t1.a as t1a, t2.a as t2a from t t2) s
> on t1.a = s.t2a + random();
>                   QUERY PLAN
> -----------------------------------------------
>  Nested Loop Left Join
>    ->  Seq Scan on t t1
>    ->  Memoize
>          Cache Key: t1.a
>          Cache Mode: binary
>          ->  Seq Scan on t t2
>                Filter: (t1.a = (a + random()))
> (7 rows)
>
> According to the theory we should not use memoize node for this query
> because of the volatile function in the inner side.  So propose a patch
> to fix that.

Thanks for the patch.  I've pushed a variation of it.

I didn't really see the need to make a single list with all the
RestrictInfos. It seems fine just to write code and loop over the
ppi_clauses checking for volatility.

David

  3. Re: Check volatile functions in ppi_clauses for memoize node

    Richard Guo <guofenglinux@gmail.com> — 2023-08-08T00:54:10Z

    On Mon, Aug 7, 2023 at 6:19 PM David Rowley <dgrowleyml@gmail.com> wrote:

> On Fri, 4 Aug 2023 at 22:26, Richard Guo <guofenglinux@gmail.com> wrote:
> > explain (costs off)
> > select * from t t1 left join lateral
> >     (select t1.a as t1a, t2.a as t2a from t t2) s
> > on t1.a = s.t2a + random();
> >                   QUERY PLAN
> > -----------------------------------------------
> >  Nested Loop Left Join
> >    ->  Seq Scan on t t1
> >    ->  Memoize
> >          Cache Key: t1.a
> >          Cache Mode: binary
> >          ->  Seq Scan on t t2
> >                Filter: (t1.a = (a + random()))
> > (7 rows)
> >
> > According to the theory we should not use memoize node for this query
> > because of the volatile function in the inner side.  So propose a patch
> > to fix that.
>
> Thanks for the patch.  I've pushed a variation of it.
>
> I didn't really see the need to make a single list with all the
> RestrictInfos. It seems fine just to write code and loop over the
> ppi_clauses checking for volatility.


That looks good.  Thanks for pushing it!

Thanks
Richard