Re: Keep notnullattrs in RelOptInfo (Was part of UniqueKey patch series)

On Fri, Mar 5, 2021 at 4:16 PM Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:

> > On Fri, Mar 05, 2021 at 10:22:45AM +0800, Andy Fan wrote:
> > > > I checked again and found I do miss the check on JoinExpr->quals. I
> have
> > > > fixed it in v3 patch. Thanks for the review!
> > > >
> > > > In the attached v3, commit 1 is the real patch, and commit 2 is
> just add
> > > > some logs to help local testing. notnull.sql/notnull.out is the test
> > > case
> > > > for
> > > > this patch, both commit 2 and notnull.* are not intended to be
> committed
> > > > at last.
> > >
> > > Just to clarify, this version of notnullattrs here is the latest one,
> > > and another one from "UniqueKey on Partitioned table" thread should be
> > > disregarded?
> > >
> >
> > Actually they are different sections for UniqueKey. Since I don't want
> to
> > mess
> > two topics in one place, I open another thread. The topic here is how to
> > represent
> > a not null attribute, which is a precondition for all UniqueKey stuff.
> The
> > thread
> > " UniqueKey on Partitioned table[1] " is talking about how to maintain
> the
> > UniqueKey on a partitioned table only.
>
> Sure, those two threads are addressing different topics. But [1] also
> includes the patch for notnullattrs (I guess it's the same as one of the
> older versions from this thread), so it would be good to specify which
> one should be used to avoid any confusion.
>
> > > I'm not sure about this, but couldn't be there still
> > > some cases when a Var belongs to nullable_baserels, but still has some
> > > constraints preventing it from being nullable (e.g. a silly example
> when
> > > the not nullable column belong to the table, and the query does full
> > > join of this table on itself using this column)?
> > >
> > > Do you say something like "SELECT * FROM t1 left join t2 on t1.a = t2.a
> > WHERE
> > t2.b = 3; "? In this case, the outer join will be reduced to inner join
> > at
> > reduce_outer_join stage, which means t2 will not be shown in
> > nullable_baserels.
>
> Nope, as I said it's a bit useless example of full self join t1 on
> itself. In this case not null column "a" will be considered as nullable,
> but following your description for is_var_nullable it's fine (although
> couple of commentaries to this function are clearly necessary).
>
> > > Is this function necessary for the following patches? I've got an
> > > impression that the discussion in this thread was mostly evolving about
> > > correct description when notnullattrs could be used, not making it
> > > bullet proof.
> > >
> >
> > Exactly, that is the blocker issue right now. I hope more authorities can
> > give
> > some suggestions to move on.
>
> Hm...why essentially a documentation question is the blocker? Or if you
> mean it's a question of the patch scope, are there any arguments for
> extending it?
>

I treat the below comment as the blocker issue:

> It would be good to agree on the correct representation for Vars that
> cannot produce NULLs so that we don't shut the door on classes of
> optimisation that require something other than what you need for your
> case.

David/Tom/Ashutosh, do you mind to share more insights to this?
I mean the target is the patch is in a committable state.

--
Best Regards
Andy Fan (https://www.aliyun.com/)