On Sat, Feb 22, 2025 at 11:32 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Richard Guo <guofenglinux@gmail.com> writes:
> > On Sat, Feb 22, 2025 at 8:51 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I've not looked at the code, but I suspect that it is failing
> >> to check varnullingrels before believing that it can trust
> >> the applicability of table constraints.
>
> > Hmm, we do check varnullingrels in expr_is_nonnullable(). My best
> > guess is that we have generated two versions of the qual 'customer.cid
> > IS NOT NULL': one with customer.cid marked as nullable by the left
> > join to customer, and one without. The latter is dropped because of
> > the not null constraint on customer.cid, while the former fails to be
> > applied on the left join to int4_tbl j.
>
> If the check against table not-null constraints is applied after we
> clone outer-join quals, that's probably bad. I think there are
> assumptions in there that every clone qual will have doppelgangers,
> so filtering NOT NULLs later would break that. Maybe not applying
> the filter to quals marked has_clone or is_clone would help?
Yeah, I think this approach can fix the issue. Perhaps we should
check whether the RestrictInfo is a clone clause and avoid assuming
that it's always true in that case, maybe by adding something like
below at the start of restriction_is_always_true.
+ if (restrictinfo->has_clone || restrictinfo->is_clone)
+ return false;
We may lose some optimization opportunities with NOT NULL quals, as
this could prevent us from reducing certain such quals to constant
true, but I think correctness should be our top priority.
I'll go ahead and write a patch.
Thanks
Richard
