Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

Поиск
Список
Период
Сортировка
От Ashutosh Bapat
Тема Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)
Дата
Msg-id CAFjFpRe8+_Jxfix=_1PgvHPb8Y2vbDgmkNVCTD+fQ3JUtbdvOA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)  (Shigeru Hanada <shigeru.hanada@gmail.com>)
Ответы Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers


On Tue, Aug 4, 2015 at 2:20 PM, Shigeru Hanada <shigeru.hanada@gmail.com> wrote:
Hi Ashutosh,

Sorry for leaving the thread.

2015-07-20 16:09 GMT+09:00 Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>:
> In find_user_mapping(), if the first cache search returns a valid tuple, it
> is checked twice for validity, un-necessarily. Instead if the first search
> returns a valid tuple, it should be returned immediately. I see that this
> code was copied from GetUserMapping(), where as well it had the same
> problem.

Oops.  I changed find_user_mapping to exit immediately when any valid
cache was found.

Thanks.
 

> In build_join_rel(), we check whether user mappings of the two joining
> relations are same. If the user mappings are same, doesn't that imply that
> the foreign server and local user are same too?

Yes, validity of umid is identical to serverid.  We can remove the
check for serverid for some cycles.  One idea is to put Assert for
serverid inside the if-statement block.

> Rest of the patch looks fine.

Thanks


I started reviewing the other patches.

In patch foreign_join_v16.patch, the user mapping structure being passed to GetConnection() is the one obtained from GetUserMappingById(). GetUserMappingById() constructs the user mapping structure from the user mapping catalog. For public user mappings, catalog entries have InvalidOid as userid. Thus, with this patch there is a chance that userid in UserMapping being passed to GetConnection(), contains InvalidOid as userid. This is not the case today. The UserMapping structure constructed using GetUserMapping(Oid userid, Oid serverid) (which ultimately gets passed to GetConnection()), has the passed in userid and not the one in the catalog. Is this change intentional?

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Simon Riggs
Дата:
Сообщение: Re: DBT-3 with SF=20 got failed
Следующее
От: Victor Wagner
Дата:
Сообщение: Re: Proposal: Implement failover on libpq connect level.