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

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(at)anarazel(dot)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