From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>, "Fujii(dot)Yuki(at)df(dot)MitsubishiElectric(dot)co(dot)jp" <Fujii(dot)Yuki(at)df(dot)mitsubishielectric(dot)co(dot)jp> |
Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>, Ian Lawrence Barwick <barwick(at)gmail(dot)com> |
Subject: | Re: Add semi-join pushdown to postgres_fdw |
Date: | 2023-01-19 17:49:14 |
Message-ID: | 4d6b5ec7-1583-f253-c6d2-810602b4b2aa@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Nathan Bossart | 2023-01-19 17:54:21 | Re: almost-super-user problems that we haven't fixed yet |
Previous Message | Tom Lane | 2023-01-19 17:29:53 | Re: minor bug |