Re: d25ea01275 and partitionwise join

On Thu, Sep 19, 2019 at 4:15 PM Amit Langote <amitlangote09(at)gmail(dot)com>
wrote:

> Hi Richard,
>
> Thanks a lot for taking a close look at the patch and sorry about the
> delay.
>
> On Wed, Sep 4, 2019 at 5:29 PM Richard Guo <riguo(at)pivotal(dot)io> wrote:
> >> On Wed, Sep 4, 2019 at 10:01 AM Amit Langote <amitlangote09(at)gmail(dot)com>
> wrote:
> > I'm reviewing v2-0002 and I have concern about how COALESCE expr is
> > processed in match_join_arg_to_partition_keys().
> >
> > If there is a COALESCE expr with first arg being non-partition key expr
> > and second arg being partition key, the patch would match it to the
> > partition key, which may result in wrong results in some cases.
> >
> > For instance, consider the partition table below:
> >
> > create table p (k int, val int) partition by range(k);
> > create table p_1 partition of p for values from (1) to (10);
> > create table p_2 partition of p for values from (10) to (100);
> >
> > So with patch v2-0002, the following query will be planned with
> > partitionwise join.
> >
> > # explain (costs off)
> > select * from (p as t1 full join p as t2 on t1.k = t2.k) as
> t12(k1,val1,k2,val2)
> > full join p as t3 on COALESCE(t12.val1,
> t12.k1) = t3.k;
> > QUERY PLAN
> > ----------------------------------------------------------
> > Append
> > -> Hash Full Join
> > Hash Cond: (COALESCE(t1.val, t1.k) = t3.k)
> > -> Hash Full Join
> > Hash Cond: (t1.k = t2.k)
> > -> Seq Scan on p_1 t1
> > -> Hash
> > -> Seq Scan on p_1 t2
> > -> Hash
> > -> Seq Scan on p_1 t3
> > -> Hash Full Join
> > Hash Cond: (COALESCE(t1_1.val, t1_1.k) = t3_1.k)
> > -> Hash Full Join
> > Hash Cond: (t1_1.k = t2_1.k)
> > -> Seq Scan on p_2 t1_1
> > -> Hash
> > -> Seq Scan on p_2 t2_1
> > -> Hash
> > -> Seq Scan on p_2 t3_1
> > (19 rows)
> >
> > But as t1.val is not a partition key, actually we cannot use
> > partitionwise join here.
> >
> > If we insert below data into the table, we will get wrong results for
> > the query above.
> >
> > insert into p select 5,15;
> > insert into p select 15,5;
>
> Good catch! It's quite wrong to use COALESCE(t12.val1, t12.k1) = t3.k
> for partitionwise join as the COALESCE expression might as well output
> the value of val1 which doesn't conform to partitioning.
>
> I've fixed match_join_arg_to_partition_keys() to catch that case and
> fail. Added a test case as well.
>
> Please find attached updated patches.
>

Thank you for the fix. Will review.

Thanks
Richard