Re: Inconsistent nullingrels due to oversight in deconstruct_distribute_oj_quals

On Fri, Feb 10, 2023 at 11:08 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> On Thu, Feb 9, 2023 at 11:55 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Richard Guo <guofenglinux(at)gmail(dot)com> writes:
>> > This query would trigger the Assert() in search_indexed_tlist_for_var.
>> > So I wonder that we should use othersj->syn_righthand here.
>>
>> There are two such calls in deconstruct_distribute_oj_quals ...
>> don't they both need this change?
>
>
> Yeah, I wondered about that too, but didn't manage to devise a query
> that can show the problem caused by the call for 'above_sjinfo' case.
> After a night of sleep I came up with one this morning. :-)
>
> create table t (a int, b int);
>
> insert into t select i, i from generate_series(1,10)i;
> analyze t;
>
> select * from t t1 left join t t2 left join t t3 on t2.b = t3.b left join
> t t4 on t2.a > t3.a on t2.a > t1.a;
>
> In this query, for the qual 't2.a > t3.a', when we try to push t3/t4
> join to above t1/t2 join, we fail to add t1/t2 ojrelid to
> nullingrels of t3.a, because t3 is not in t1/t2 join's min_righthand
> (but in its syn_righthand). We really should have done that because
> after the commutation t1/t2 join can null not only t2 but also t3 in
> this case.
>

However, for 'above_sjinfo' case, we should not use
othersj->syn_righthand, because othersj->syn_righthand contains relids
in sjinfo's righthand which should not be nulled by othersj after the
commutation. It seems what we should use here is sjinfo->syn_lefthand.

--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -1990,7 +1990,7 @@ deconstruct_distribute_oj_quals(PlannerInfo *root,
if (above_sjinfo)
quals = (List *)
add_nulling_relids((Node *) quals,
- othersj->min_righthand,
+ sjinfo->syn_lefthand,
bms_make_singleton(othersj->ojrelid));

Thanks
Richard