Re: An incorrect check in get_memoize_path - Mailing list pgsql-hackers

While reviewing another patch [1], I noticed an incorrect check in
get_memoize_path.

    if (extra->inner_unique &&
        (inner_path->param_info == NULL ||
         bms_num_members(inner_path->param_info->ppi_serials) <
         list_length(extra->restrictlist)))
        return NULL;

This check is to ensure that, for unique joins, all the restriction
clauses are parameterized to enable the use of Memoize nodes.
However, ppi_clauses includes join clauses available from all outer
rels, not just the current outer rel, while extra->restrictlist only
includes the restriction clauses for the current join.  This means the
check could pass even if a restriction clause isn't parameterized, as
long as another join clause, which doesn't belong to the current join,
is included in ppi_clauses.  It's not very hard to construct a query
that exposes this issue.

create table t (a int, b int);

explain (costs off)
select * from t t0 left join
  (t t1 left join t t2 on t1.a = t2.a
   join lateral
     (select distinct on (a, b) a, b, t0.a+t1.a+t2.a as x from t t3 offset 0) t3
         on t1.a = t3.a and coalesce(t2.b) = t3.b)
  on t0.b = t3.b;
                       QUERY PLAN
---------------------------------------------------------
 Nested Loop Left Join
   ->  Seq Scan on t t0
   ->  Nested Loop
         Join Filter: (COALESCE(t2.b) = t3.b)
         ->  Hash Left Join
               Hash Cond: (t1.a = t2.a)
               ->  Seq Scan on t t1
               ->  Hash
                     ->  Seq Scan on t t2
         ->  Subquery Scan on t3
               Filter: ((t0.b = t3.b) AND (t1.a = t3.a))
               ->  Unique
                     ->  Sort
                           Sort Key: t3_1.a, t3_1.b
                           ->  Seq Scan on t t3_1
(15 rows)

Consider the join to t3.  It is a unique join, and not all of its
restriction clauses are parameterized.  Despite this, the check still
passes.

Interestingly, although the check passes, no Memoize nodes are added
on top of t3's paths because paraminfo_get_equal_hashops insists that
all ppi_clauses must satisfy clause_sides_match_join (though I
actually question whether this is necessary, but that's a separate
issue).  However, this does not justify the check being correct.

Hence, I propose the attached patch for the fix.

BTW, there is a XXX comment there saying that maybe we can make the
remaining join quals part of the inner scan's filter instead of the
join filter.  I don't think this is possible in all cases.  In the
above query, 'coalesce(t2.b) = t3.b' cannot be made part of t3's scan
filter, according to join_clause_is_movable_into.  So I removed that
comment in the patch while we're here.

Any thoughts?

[1] https://postgr.es/m/60bf8d26-7c7e-4915-b544-afdb9020011d@gmail.com

Thanks
Richard