Re: postgres_fdw: wrong results with self join + enable_nestloop off

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Cc: Nishant Sharma <nishant(dot)sharma(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: postgres_fdw: wrong results with self join + enable_nestloop off
Date: 2023-04-25 10:06:01
Message-ID: CAMbWs49giYXU3HHdMbNJ+rb+brH6HaW88o-csqbBXNGhQ-Q=1Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Apr 14, 2023 at 8:51 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
wrote:

> I think that the root cause for this issue would be in the
> create_scan_plan handling of pseudoconstant quals when creating a
> foreign-join (or custom-join) plan.

Yes exactly. In create_scan_plan, we are supposed to extract all the
pseudoconstant clauses and use them as one-time quals in a gating Result
node. Currently we check against rel->baserestrictinfo and ppi_clauses
for the pseudoconstant clauses. But for scans of foreign joins, we do
not have any restriction clauses in these places and thus the gating
Result node as well as the pseudoconstant clauses would just be lost.

I looked at Nishant's patch. IIUC it treats the pseudoconstant clauses
as local conditions. While it can fix the wrong results issue, I think
maybe it's better to still treat the pseudoconstant clauses as one-time
quals in a gating node. So I wonder if we can store the restriction
clauses for foreign joins in ForeignPath, just as what we do for normal
JoinPath, and then check against them for pseudoconstant clauses in
create_scan_plan, something like attached.

BTW, while going through the codes I noticed one place in
add_foreign_final_paths that uses NULL for List *. I changed it to NIL.

Thanks
Richard

Attachment Content-Type Size
v1-0001-Fix-how-we-handle-pseudoconstant-clauses-for-scans-of-foreign-joins.patch application/octet-stream 10.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-04-25 10:27:52 Re: ssl tests aren't concurrency safe due to get_free_port()
Previous Message Alvaro Herrera 2023-04-25 09:20:40 Re: Missing update of all_hasnulls in BRIN opclasses