Thread: Obsolete comment in postgres_fdw.c

Hi,

Here is the comments for foreign_join_ok in postgres_fdw.c:

/*
 * Assess whether the join between inner and outer relations can be
pushed down
 * to the foreign server. As a side effect, save information we obtain
in this
 * function to PgFdwRelationInfo passed in.
 *
 * Joins that satisfy conditions below are safe to push down.
 *
 * 1) Join type is INNER or OUTER (one of LEFT/RIGHT/FULL)
 * 2) Both outer and inner portions are safe to push-down
 * 3) All foreign tables in the join belong to the same foreign server
and use
 *    the same user mapping.
 * 4) All join conditions are safe to push down
 * 5) No relation has local filter (this can be relaxed for INNER JOIN,
if we
 *    can move unpushable clauses upwards in the join tree).
 */

The condition 3 is now checked by the core, so I'd like to remove that
condition from the above comments.

In addition, I'd like to update some related comments in
src/include/nodes/relation.h and src/backend/optimizer/path/joinpath.c.

Attached is a patch for that.

Best regards,
Etsuro Fujita

On 2016/03/14 16:42, Ashutosh Bapat wrote:
> On Mon, Mar 14, 2016 at 9:05 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp <mailto:fujita.etsuro@lab.ntt.co.jp>> wrote:

>     Here is the comments for foreign_join_ok in postgres_fdw.c:
>
>     /*
>       * Assess whether the join between inner and outer relations can be
>     pushed down
>       * to the foreign server. As a side effect, save information we obtain
>     in this
>       * function to PgFdwRelationInfo passed in.
>       *
>       * Joins that satisfy conditions below are safe to push down.
>       *
>       * 1) Join type is INNER or OUTER (one of LEFT/RIGHT/FULL)
>       * 2) Both outer and inner portions are safe to push-down
>       * 3) All foreign tables in the join belong to the same foreign server
>     and use
>       *    the same user mapping.
>       * 4) All join conditions are safe to push down
>       * 5) No relation has local filter (this can be relaxed for INNER JOIN,
>     if we
>       *    can move unpushable clauses upwards in the join tree).
>       */
>
>     The condition 3 is now checked by the core, so I'd like to remove that
>     condition from the above comments.

> It was left there intentionally to document all the conditions in one
> place (some from the core and some from the FDW itself), for a ready
> reference. In case tomorrow core thinks that matching user mapping is
> not required, postgres_fdw would still require it to be incorporated.

Thank you for the explanation!  I understand the reason, but that seems 
confusing to me.

>     In addition, I'd like to update some related comments in
>     src/include/nodes/relation.h and src/backend/optimizer/path/joinpath.c.
>
>
> Those look fine. Sorry for missing those in the commit and thanks for
> providing a patch for the same.

No problem.

Best regards,
Etsuro Fujita

On Mon, Mar 14, 2016 at 4:31 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
>> It was left there intentionally to document all the conditions in one
>> place (some from the core and some from the FDW itself), for a ready
>> reference. In case tomorrow core thinks that matching user mapping is
>> not required, postgres_fdw would still require it to be incorporated.
>
> Thank you for the explanation!  I understand the reason, but that seems
> confusing to me.

Agreed.

>>     In addition, I'd like to update some related comments in
>>     src/include/nodes/relation.h and
>> src/backend/optimizer/path/joinpath.c.
>>
>>
>> Those look fine. Sorry for missing those in the commit and thanks for
>> providing a patch for the same.
>
> No problem.

Committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company