Remove an unnecessary check on semijoin_target_ok() on postgres_fdw.c

Tender Wang писал(а) 2024-10-09 10:26:

Hi,
When I debug FDW join pushdown codes, I found below codes in
semijoin_target_ok():
if (bms_is_member(var->varno, innerrel->relids) &&

!bms_is_member(var->varno, outerrel->relids))

As far as I know, if a var belongs to the innerrel of joinrel, it's
not possible that it
may belong to the outerrel. So if the bms_is_member(var->varno,
innerrel->relids)
returns TRUE, then !bms_is_member(var->varno, outerrel->relids) must
be TRUE.
If bms_is_member(var->varno, innerrel->relids) returns FALSE,
!bms_is_member(var->varno, outerrel->relids) will not execute due to
short circuit.

So I think we can remove the "!bms_is_member(var->varno,
outerrel->relids)" from if.
Any thoughts?

Hi.
Seems good to me.
--
Best regards,
Alexander Pyhalov,
Postgres Professional

Alexander Pyhalov <a.pyhalov@postgrespro.ru> 于2024年11月29日周五 00:02写道：

Tender Wang писал(а) 2024-10-09 10:26:

Hi,
When I debug FDW join pushdown codes, I found below codes in
semijoin_target_ok():
if (bms_is_member(var->varno, innerrel->relids) &&

!bms_is_member(var->varno, outerrel->relids))

As far as I know, if a var belongs to the innerrel of joinrel, it's
not possible that it
may belong to the outerrel. So if the bms_is_member(var->varno,
innerrel->relids)
returns TRUE, then !bms_is_member(var->varno, outerrel->relids) must
be TRUE.
If bms_is_member(var->varno, innerrel->relids) returns FALSE,
!bms_is_member(var->varno, outerrel->relids) will not execute due to
short circuit.

So I think we can remove the "!bms_is_member(var->varno,
outerrel->relids)" from if.
Any thoughts?

Hi.
Seems good to me.
--
Best regards,
Alexander Pyhalov,
Postgres Professional

Thanks for looking at that.

--
Thanks,
Tender Wang

On Fri, Nov 29, 2024 at 3:39 AM Tender Wang <tndrwang@gmail.com> wrote:

Alexander Pyhalov <a.pyhalov@postgrespro.ru> 于2024年11月29日周五 00:02写道：

Tender Wang писал(а) 2024-10-09 10:26:

Hi,
When I debug FDW join pushdown codes, I found below codes in
semijoin_target_ok():
if (bms_is_member(var->varno, innerrel->relids) &&

!bms_is_member(var->varno, outerrel->relids))

As far as I know, if a var belongs to the innerrel of joinrel, it's
not possible that it
may belong to the outerrel. So if the bms_is_member(var->varno,
innerrel->relids)
returns TRUE, then !bms_is_member(var->varno, outerrel->relids) must
be TRUE.
If bms_is_member(var->varno, innerrel->relids) returns FALSE,
!bms_is_member(var->varno, outerrel->relids) will not execute due to
short circuit.

So I think we can remove the "!bms_is_member(var->varno,
outerrel->relids)" from if.
Any thoughts?

Hi.
Seems good to me.
--
Best regards,
Alexander Pyhalov,
Postgres Professional

Thanks for looking at that.

Pushed. But I've decided to keep the redundant check as an assertion.

------
Regards,
Alexander Korotkov
Supabase

Alexander Korotkov <aekorotkov@gmail.com> 于2025年3月25日周二 18:57写道：

On Fri, Nov 29, 2024 at 3:39 AM Tender Wang <tndrwang@gmail.com> wrote:

Alexander Pyhalov <a.pyhalov@postgrespro.ru> 于2024年11月29日周五 00:02写道：

Tender Wang писал(а) 2024-10-09 10:26:

Hi,
When I debug FDW join pushdown codes, I found below codes in
semijoin_target_ok():
if (bms_is_member(var->varno, innerrel->relids) &&

!bms_is_member(var->varno, outerrel->relids))

As far as I know, if a var belongs to the innerrel of joinrel, it's
not possible that it
may belong to the outerrel. So if the bms_is_member(var->varno,
innerrel->relids)
returns TRUE, then !bms_is_member(var->varno, outerrel->relids) must
be TRUE.
If bms_is_member(var->varno, innerrel->relids) returns FALSE,
!bms_is_member(var->varno, outerrel->relids) will not execute due to
short circuit.

So I think we can remove the "!bms_is_member(var->varno,
outerrel->relids)" from if.
Any thoughts?

Hi.
Seems good to me.
--
Best regards,
Alexander Pyhalov,
Postgres Professional

Thanks for looking at that.

Pushed. But I've decided to keep the redundant check as an assertion.

Thanks for pushing.

--
Thanks,
Tender Wang