Re: Add semi-join pushdown to postgres_fdw

Alexander Korotkov писал(а) 2023-12-03 23:52:
> Hi, Alexander!
>
> On Mon, Nov 27, 2023 at 5:11 PM Alexander Pyhalov
> <a(dot)pyhalov(at)postgrespro(dot)ru> wrote:
>> Alexander Korotkov писал(а) 2023-11-27 03:49:
>>
>> > Thank you for the revision.
>> >
>> > I've revised the patch myself. I've replaced StringInfo with
>> > additional conds into a list of strings as I proposed before. I think
>> > the code became much clearer. Also, it gets rid of some unnecessary
>> > allocations.
>> >
>> > I think the code itself is not in bad shape. But patch lacks some
>> > high-level description of semi-joins processing as well as comments on
>> > each manipulation with additional conds. Could you please add this?
>> >
>>
>> Hi. The updated patch looks better. It seems I've failed to fix logic
>> in
>> deparseFromExprForRel() when tried to convert StringInfos to Lists.
>>
>> I've added some comments. The most complete description of how
>> SEMI-JOIN
>> is processed, is located in deparseFromExprForRel(). Unfortunately,
>> there seems to be no single place, describing current JOIN deparsing
>> logic.
>
> Looks good to me. I've made some grammar and formatting adjustments.
> Also, I've written the commit message.
>
> Now, I think this looks good. I'm going to push this if no objections.
>
> ------
> Regards,
> Alexander Korotkov

Hi. No objections from my side.

Perhaps, some rephrasing is needed in comment in semijoin_target_ok():

"The planner can create semi-joins, which refer to inner rel
vars in its target list."

Perhaps, change "semi-joins, which refer" to "a semi-join, which refers
...",
as later we speak about "its" target list.

--
Best regards,
Alexander Pyhalov,
Postgres Professional