Re: Odd system-column handling in postgres_fdw join pushdown patch

Поиск
Список
Период
Сортировка
От Ashutosh Bapat
Тема Re: Odd system-column handling in postgres_fdw join pushdown patch
Дата
Msg-id CAFjFpResJvUhN6vum86YMMe1fHfNwqqZMvROpyWsGVjBEEq5Uw@mail.gmail.com
обсуждение исходный текст
Ответ на Odd system-column handling in postgres_fdw join pushdown patch  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
Ответы Re: Odd system-column handling in postgres_fdw join pushdown patch  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
Список pgsql-hackers


On Thu, Mar 17, 2016 at 4:30 PM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:
Hi,

I noticed that the postgres_fdw join pushdown patch retrieves system
columns other than ctid (and oid) from the remote server as shown in the
example:

postgres=# explain verbose select foo.tableoid, foo.xmin, foo.cmin,
foo.xmax, foo.cmax, foo.* from foo, bar where foo.a = bar.a;

   QUERY PLAN

--------------------------------------------------------------------------------------------------------------------------------------------------------
--------
 Foreign Scan  (cost=100.00..102.09 rows=2 width=28)
   Output: foo.tableoid, foo.xmin, foo.cmin, foo.xmax, foo.cmax, foo.a,
foo.b
   Relations: (public.foo) INNER JOIN (public.bar)
   Remote SQL: SELECT r1.tableoid, r1.xmin, r1.cmin, r1.xmax, r1.cmax,
r1.a, r1.b FROM (public.foo r1 INNER JOIN public.bar r2 ON (TRUE)) WHERE
((r1.a =
 r2.a))
(4 rows)

Thanks for catching the bug and producing a patch.
 

BUT: we don't make any effort to ensure that local and remote values
match, so system columns other than ctid and oid should not be retrieved
from the remote server.  So, I'd like to propose: (1) when tableoids are
requested from the remote server, postgres_fdw sets valid values for
them locally, instead (core should support that?) and

If we are disabling join pushdown when the targetlist has other system columns, shouldn't we treat tableoid in the same fashion. We should disable join pushdown when tableoid is requested?

I agree that we might want to do this in core instead of FDW specific core. That way we avoid each FDW implementing its own solution. Ultimately, all that needs to be done to push OID of the foreign table in place of tableoid column. The core code can do that. It already does that for the base tables.
 
(2) when any of
xmins, xmaxs, cmins, and cmaxs are requested, postgres_fdw gives up
pushing down foreign joins.  (We might be able to set appropriate values
for them locally the same way as for tableoids, but I'm not sure it's
worth complicating the code.)  I think that would be probably OK,
because users wouldn't retrieve any such columns in practice.

In that patch you have set pushdown_safe to false for the base relation fetching system columns. But pushdown_safe = false means that that relation is not safe to push down. A base foreign relation is always safe to push down, so should have pushdown_safe = true always. Instead, I suggest having a separate boolean has_unshippable_syscols (or something with similar name) in PgFdwRelationInfo, which is set to true in such case. In foreign_join_ok, we return false (thus not pushing down the join), if any of the joining relation has that attribute set. By default this member is false.

Even for a base table those values are rather random, although they are not fetched from the foreign server. Instead of not pushing the join down, we should push the join down without fetching those attributes. While constructing the query, don't include these system attributes in SELECT clause and don't include corresponding positions in retrieved_attributes list. That means those attributes won't be set while fetching the row from the foreign server and will have garbage values in corresponding places. I guess that would work.

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

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

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: [BUGS] pgbench -C -M prepared gives an error
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: [BUGS] pgbench -C -M prepared gives an error