public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alexander Pyhalov <[email protected]>
To: Alexander Korotkov <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: Pgsql Hackers <[email protected]>
Cc: Ian Lawrence Barwick <[email protected]>
Subject: Re: Add semi-join pushdown to postgres_fdw
Date: Tue, 05 Dec 2023 13:29:47 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAPpHfdsbqy93Og9_0UrtNmCxE7LRwLWLTrmeHkOfeWtvg5tSAA@mail.gmail.com>
References: <[email protected]>
	<CAExHW5u+wy7YAp61o-QJ=p8CMjZCZbcin=_xh7bYjeyJV1a1pg@mail.gmail.com>
	<[email protected]>
	<CAB8KJ=hXB8=JfFM5e+UTkEDTNU0gA8Xr-nToMNGahZc7ktD-gA@mail.gmail.com>
	<[email protected]>
	<OS3PR01MB6660A7F36A2D37194DB601F195169@OS3PR01MB6660.jpnprd01.prod.outlook.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAPpHfdtLSp3YnmbTMpUZvBa=kJ3qLosUQVjNfW3__=oWUP8dkg@mail.gmail.com>
	<[email protected]>
	<CAPpHfdtYndP0+P4YjJrNAgX6X6xVL6z4n0L_u3P=se+Xt0VksA@mail.gmail.com>
	<[email protected]>
	<CAPpHfdsbqy93Og9_0UrtNmCxE7LRwLWLTrmeHkOfeWtvg5tSAA@mail.gmail.com>

Alexander Korotkov писал(а) 2023-12-03 23:52:
> Hi, Alexander!
> 
> On Mon, Nov 27, 2023 at 5:11 PM Alexander Pyhalov
> <[email protected]> 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






view thread (2+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add semi-join pushdown to postgres_fdw
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox