public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tomas Vondra <[email protected]>
To: Alexander Pyhalov <[email protected]>
To: [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: Thu, 19 Jan 2023 18:49:14 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
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]>

Hi.

I took a quick look at the patch. It needs a rebase, although it applies
fine using patch.

A couple minor comments:

1) addl_conds seems a bit hard to understand, I'd use either the full
wording (additional_conds) or maybe extra_conds

2) some of the lines got quite long, and need a wrap

3) unknown_subquery_rels name is a bit misleading - AFAIK it's the rels
that can't be referenced from upper rels (per what the .h says). So they
are known, but hidden. Is there a better name?

4) joinrel_target_ok() needs a better comment, explaining *when* the
reltarget is safe for pushdown. The conditions are on the same row, but
the project style is to break after '&&'.

Also, I'd write

    if (!IsA(var, Var))
        continue;

which saves one level of nesting. IMHO that makes it more readable.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

view thread (19+ 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]
  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