Re: Re: [COMMITTERS] pgsql: Only try to push down foreign joins if the user mapping OIDs mat - Mailing list pgsql-hackers

From Etsuro Fujita
Subject Re: Re: [COMMITTERS] pgsql: Only try to push down foreign joins if the user mapping OIDs mat
Date
Msg-id 56E6240E.5090301@lab.ntt.co.jp
Whole thread Raw
In response to Re: [COMMITTERS] pgsql: Only try to push down foreign joins if the user mapping OIDs mat  (Andres Freund <andres@anarazel.de>)
Responses Re: Re: [COMMITTERS] pgsql: Only try to push down foreign joins if the user mapping OIDs mat
List pgsql-hackers
Hi,

On 2016/03/13 4:46, Andres Freund wrote:
> On 2016-03-12 11:56:24 -0500, Robert Haas wrote:
>> On Fri, Mar 11, 2016 at 10:15 PM, Andres Freund <andres@anarazel.de> wrote:
>>> On 2016-01-28 19:09:01 +0000, Robert Haas wrote:
>>>> Only try to push down foreign joins if the user mapping OIDs match.
>>>>
>>>> Previously, the foreign join pushdown infrastructure left the question
>>>> of security entirely up to individual FDWs, but it would be easy for
>>>> a foreign data wrapper to inadvertently open up subtle security holes
>>>> that way.  So, make it the core code's job to determine which user
>>>> mapping OID is relevant, and don't attempt join pushdown unless it's
>>>> the same for all relevant relations.
>>>>
>>>> Per a suggestion from Tom Lane.  Shigeru Hanada and Ashutosh Bapat,
>>>> reviewed by Etsuro Fujita and KaiGai Kohei, with some further
>>>> changes by me.

>>> I noticed that this breaks some citus regression tests in a minor
>>> manner. Namely previously file_fdw worked without a user mapping, now it
>>> doesn't appear to anymore.
>>>
>>> This is easy enough to fix, and it's perfectly ok for us to fix this,
>>> but I do wonder if that's not going to cause trouble for others.

>> Hmm, I didn't intend to change that.  If that commit broke something,
>> there's obviously a hole in our regression test coverage.

> CREATE EXTENSION file_fdw;
> CREATE SERVER file_server FOREIGN DATA WRAPPER file_fdw;
>
> CREATE FOREIGN TABLE agg_csv (
>          a       int2,
>          b       float4
> ) SERVER file_server
> OPTIONS (format 'csv', filename '/home/andres/src/postgresql/contrib/file_fdw/data/agg.csv', header 'true', delimiter
';',quote '@', escape '"', null '');
 
>
> SELECT * FROM agg_csv;

> worked in 9.5, but doesn't in master. The difference apears to be the
> check that's now in build_simple_rel() - there was nothing hitting the
> user mapping code before for file_fdw.

Exactly.

I'm not sure it's worth complicating the code to keep that behavior, so 
I'd vote for adding the change notice to 9.6 release notes or something 
like that in addition to updating file-fdw.sgml.

Best regards,
Etsuro Fujita





pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Parallel Aggregate
Next
From: Tom Lane
Date:
Subject: Re: Re: [COMMITTERS] pgsql: Only try to push down foreign joins if the user mapping OIDs mat