Re: postgres_fdw: support parameterized foreign joins

From: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
To: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw: support parameterized foreign joins
Date: 2017-04-04 15:55:14
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 23.03.2017 15:45, Etsuro Fujita wrote:
> Done. Also, I added regression tests and revised code and comments a
> bit. (As for create_foreignscan_path(), I haven't done anything about
> that yet.) Please find attached a new version created on top of [1].

Thank you!

I didn't notice that it is necessary to apply the patch

I have a few comments.

> * innersortkeys are the sort pathkeys for the inner side of the mergejoin
> + * req_outer_list is a list of sensible parameterizations for the join rel
> */

I think it would be better if the comment explained what type is stored
in req_outer_list. So the following comment would be good:

"req_outer_list is a list of Relids of sensible parameterizations for
the join rel"

> ! Assert(foreignrel->reloptkind == RELOPT_JOINREL);
> !

Here the new macro IS_JOIN_REL() can be used.

> ! /* Get the remote and local conditions */
> ! remote_conds = list_concat(list_copy(remote_param_join_conds),
> ! fpinfo->remote_conds);
> ! local_param_join_exprs =
> ! get_actual_clauses(local_param_join_conds);
> ! local_exprs = list_concat(list_copy(local_param_join_exprs),
> ! fpinfo->local_conds);

Is this code correct? 'remote_conds' and 'local_exprs' are initialized
above when 'scan_clauses' is separated. Maybe better to use
'remote_conds' and 'local_exprs' instead of 'fpinfo->remote_conds' and
'fpinfo->local_conds' respectively?

And the last. The patch needs rebasing because new macroses
IS_JOIN_REL() and IS_UPPER_REL() were added. And the patch is applied
with errors.

Arthur Zakirov
Postgres Professional:
Russian Postgres Company

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Claudio Freire 2017-04-04 15:58:22 Re: Making clausesel.c Smarter
Previous Message Stephen Frost 2017-04-04 15:42:57 Re: PATCH: Make pg_stop_backup() archive wait optional