Re: d25ea01275 and partitionwise join

On Sat, Apr 4, 2020 at 6:13 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> > Updated patches attached.
>
> I looked through these and committed 0001+0002, with some further
> comment-polishing. However, I have no faith at all in 0003.

Thanks for the review.

> It is
> blithely digging through COALESCE expressions with no concern for
> whether they came from full joins or not, or whether the other values
> being coalesced to might completely change the semantics. Digging
> through PlaceHolderVars scares me even more; what's that got to do
> with the problem, anyway? So while this might fix the complained-of
> issue of failing to use a partitionwise join, I think it wouldn't be
> hard to create examples that it would incorrectly turn into
> partitionwise joins.
>
> I wonder whether it'd be feasible to fix the problem by going in the
> other direction; that is, while constructing the nullable_partexprs
> lists for a full join, add synthesized COALESCE() expressions for the
> output columns (by wrapping COALESCE around copies of the input rels'
> partition expressions), and then not need to do anything special in
> match_expr_to_partition_keys. We'd still need to convince ourselves
> that this did the right thing and not any of the wrong things, but
> I think it might be easier to prove it that way.

Okay, I tried that in the updated patch. I didn't try to come up with
examples that might break it though.

--
Thank you,

Amit Langote
EnterpriseDB: http://www.enterprisedb.com