Re: WIP: Join push-down for foreign tables

Kaigai-san,

Thanks for the review.

(2011/10/03 17:07), Kohei KaiGai wrote:
> At first, I tried to use file_fdw, however, it was crashed of course.
> It seems to me this logic should be modified to confirm whether the target FDW
> support join push down, or not.
>
> + if (enable_foreignjoin&&
> + joinrel->serverid != InvalidOid&&
> + (IsA(outerpath, ForeignPath) || IsA(outerpath, ForeignJoinPath))&&
> + (IsA(inner_cheapest_total, ForeignPath) ||
> + IsA(inner_cheapest_total, ForeignJoinPath)))
> +
> + {
> + ForeignJoinPath *path;
> + path = create_foreignjoin_path(root,
> + joinrel,
> + jointype,
> + sjinfo,
> + outerpath,
> + inner_cheapest_total,
> + restrictlist,
> + merge_pathkeys);
> + if (path != NULL)
> + add_path(joinrel, (Path *) path);
> + }
> +
>
> In my opinion, FdwRoutine should have an additional API to inform the core its
> supported features; such as inner-join, outer-join, order-by,
> group-by, aggregate
> functions, insert, update, delete, etc... in the future version.

Sure, so in my design PlanForeignJoin is optional.

The lack of capability is informed from FDW with setting function
pointer in FdwRoutine to NULL. If PlanForeignJoin was NULL, core
(planner) will give up to consider join push-down, and use one of local
join methods such as NestLoop and MergeJoin for those foreign tables.
As you say, other push-down-able features would also have optional
handler function for each.

BTW, what is the point of separating inner-join and outer-join in
context of push-down? I think that providing the type of the join to
FDW as parameter of PlanForeignJoin is enough. Then they can tell core
planner to give up considering join push-down by returning NULL if the
type of the join was not supported.

> Obviously, it is not hard to implement inner/outer-join feature for
> pgsql_fdw module,
> but it may be a tough work for memcached_fdw module.

Agreed, join push-down might be useful for only RDBMS wrappers. NoSQL
wrapper would not provide handler functions other than required ones for
simple scan.

>> *) SELECT * FROM A JOIN B (...) doesn't work. Specifying columns in
>> SELECT clause explicitly like "SELECT A.col1, A.col2, ..." seems to work.
>> *) ORDER BY causes error if no column is specified in SELECT clause from
>> sort key's table.
>>
> I doubt these issues are in pgsql_fdw side, not the proposed patch itself.

Yes, this problem is from pgsql_fdw's SQL generator. I'll fix it.

> In the case when the table and column names/types are compatible between
> local-side and remote-side, the problem was not reproduced in my environment.
> I'd like to suggest you to add a functionality to map remote column names to
> the local ones in pgsql_fdw.

Since per-column FDW options patch has been committed in last CF, It's
not hard to implement colname FDW option for pgsql_fdw, and rough
implementation has been already done. In next post you will be able to
map column names. :)

Regards,
--
Shigeru Hanada