Re: d25ea01275 and partitionwise join

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.

Thanks,
Amit