Re: postgres_fdw: support parameterized foreign joins

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
"epqpath-for-foreignjoin".

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: http://www.postgrespro.com
Russian Postgres Company