MIME-Version: 1.0
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 9 Feb 2023 17:16:15 +0800
Message-ID: <CAMbWs497CmBruMx1SOjepWEz+T5NWa4scqbdE9v7ZzSXqH_gQw@mail.gmail.com>
Subject: Inconsistent nullingrels due to oversight in deconstruct_distribute_oj_quals
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000fdcb3505f440d570"
Archived-At: <https://www.postgresql.org/message-id/CAMbWs497CmBruMx1SOjepWEz%2BT5NWa4scqbdE9v7ZzSXqH_gQw%40mail.gmail.com>
Precedence: bulk

--000000000000fdcb3505f440d570
Content-Type: text/plain; charset="UTF-8"

When we try to generate qual variants with different nullingrels in
deconstruct_distribute_oj_quals, we traverse all the JoinTreeItems and
adjust qual nulling bits as we crawl up the join tree.  For a
SpecialJoinInfo which commutes with current sjinfo from below left, in
the next level up it would null all the relids in its righthand.  So we
adjust qual nulling bits as below.

     /*
      * Adjust qual nulling bits for next level up, if needed.  We
      * don't want to put sjinfo's own bit in at all, and if we're
      * above sjinfo then we did it already.
      */
     if (below_sjinfo)
         quals = (List *)
             add_nulling_relids((Node *) quals,
                                othersj->min_righthand,
                                bms_make_singleton(othersj->ojrelid));

It seems to me there is oversight here.  Actually in next level up this
othersj would null all the relids in its syn_righthand, not only the
relids in its min_righthand.   If the quals happen to contain references
to relids which are in othersj->syn_righthand but not in
othersj->min_righthand, these relids would not get updated with
othersj->ojrelid added.  And this would cause qual nulling bits not
consistent.

I've managed to devise a query that can show this problem.

create table t1(a int, b int);
create table t2(a int, b int);
create table t3(a int, b int);
create table t4(a int, b int);

insert into t1 select i, i from generate_series(1,10)i;
insert into t2 select i, i from generate_series(1,10)i;
insert into t3 select i, i from generate_series(1,1000)i;
insert into t4 select i, i from generate_series(1,1000)i;
analyze;

select * from t1 left join (t2 left join t3 on t2.a > t3.a) on t1.b = t2.b
left join t4 on t2.b = t3.b;

This query would trigger the Assert() in search_indexed_tlist_for_var.
So I wonder that we should use othersj->syn_righthand here.

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

Thanks
Richard

--000000000000fdcb3505f440d570
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">When we try to generate qual variants with different nulli=
ngrels in<br>deconstruct_distribute_oj_quals, we traverse all the JoinTreeI=
tems and<br>adjust qual nulling bits as we crawl up the join tree.=C2=A0 Fo=
r a<br>SpecialJoinInfo which commutes with current sjinfo from below left, =
in<br>the next level up it would null all the relids in its righthand.=C2=
=A0 So we<br>adjust qual nulling bits as below.<br><br>=C2=A0 =C2=A0 =C2=A0=
/*<br>=C2=A0 =C2=A0 =C2=A0 * Adjust qual nulling bits for next level up, if=
 needed.=C2=A0 We<br>=C2=A0 =C2=A0 =C2=A0 * don&#39;t want to put sjinfo=
9;s own bit in at all, and if we&#39;re<br>=C2=A0 =C2=A0 =C2=A0 * above sji=
nfo then we did it already.<br>=C2=A0 =C2=A0 =C2=A0 */<br>=C2=A0 =C2=A0 =C2=
=A0if (below_sjinfo)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0quals =3D (List *=
)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0add_nulling_relids((No=
de *) quals,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 othersj-&gt;min_righth=
and,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bms_make_singleton(othersj-&g=
t;ojrelid));<br><br>It seems to me there is oversight here.=C2=A0 Actually =
in next level up this<br>othersj would null all the relids in its syn_right=
hand, not only the<br>relids in its min_righthand. =C2=A0 If the quals happ=
en to contain references<br>to relids which are in othersj-&gt;syn_righthan=
d but not in<br>othersj-&gt;min_righthand, these relids would not get updat=
ed with<br>othersj-&gt;ojrelid added.=C2=A0 And this would cause qual nulli=
ng bits not<br>consistent.<br><br>I&#39;ve managed to devise a query that c=
an show this problem.<br><br>create table t1(a int, b int);<br>create table=
 t2(a int, b int);<br>create table t3(a int, b int);<br>create table t4(a i=
nt, b int);<br><br>insert into t1 select i, i from generate_series(1,10)i;<=
br>insert into t2 select i, i from generate_series(1,10)i;<br>insert into t=
3 select i, i from generate_series(1,1000)i;<br>insert into t4 select i, i =
from generate_series(1,1000)i;<br>analyze;<br><br>select * from t1 left joi=
n (t2 left join t3 on t2.a &gt; t3.a) on t1.b =3D t2.b left join t4 on t2.b=
 =3D t3.b;<br><br>This query would trigger the Assert() in search_indexed_t=
list_for_var.<br><div>So I wonder that we should use othersj-&gt;syn_righth=
and here.<br><br>--- a/src/backend/optimizer/plan/initsplan.c<br>+++ b/src/=
backend/optimizer/plan/initsplan.c<br>@@ -2046,7 +2046,7 @@ deconstruct_dis=
tribute_oj_quals(PlannerInfo *root,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (belo=
w_sjinfo)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 quals =3D (List *)<b=
r>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 add_nulling_relid=
s((Node *) quals,<br>- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0othersj-&=
gt;min_righthand,<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0othersj-&=
gt;syn_righthand,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0bm=
s_make_singleton(othersj-&gt;ojrelid));<br><br>Thanks<br>Richard<br></div><=
/div>

--000000000000fdcb3505f440d570--