Re: Postgres_fdw join pushdown - wrong results with whole-row reference

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Postgres_fdw join pushdown - wrong results with whole-row reference
Date: 2016-06-27 16:54:35
Message-ID: CA+TgmoaC6f7Cm_Z-5-dcHtdvwdrOJyqDg6=H8hJUTTWn69JQLQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 27, 2016 at 2:47 AM, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> On Sat, Jun 25, 2016 at 12:44 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Wed, Jun 22, 2016 at 4:11 AM, Amit Langote
>> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> >> In an outer join we have to differentiate between a row being null
>> >> (because
>> >> there was no joining row on nullable side) and a non-null row with all
>> >> column values being null. If we cast the whole-row expression to a text
>> >> e.g. r.*::text and test the resultant value for nullness, it gives us
>> >> what
>> >> we want. A null row casted to text is null and a row with all null
>> >> values
>> >> casted to text is not null.
>> >
>> > You are right. There may be non-null rows with all columns null which
>> > are
>> > handled wrongly (as Rushabh reports) and the hack I proposed is not
>> > right
>> > for. Especially if from non-nullable side as in the reported case, NULL
>> > test for such a whole-row-var would produce the wrong result. Casting
>> > to
>> > text as your patch does produces the correct behavior.
>>
>> I agree, but I think we'd better cast to pg_catalog.text instead, just
>> to be safe. Committed that way.
>
> postgres_fdw resets the search path to pg_catalog while opening connection
> to the server. The reason behind this is explained in deparse.c
>
> * We assume that the remote session's search_path is exactly "pg_catalog",
> * and thus we need schema-qualify all and only names outside pg_catalog.

Hmm. OK, should we revert the schema-qualification part of that
commit, or just leave it alone?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2016-06-27 17:05:11 Re: Rename max_parallel_degree?
Previous Message Robert Haas 2016-06-27 16:53:03 Re: parallel workers and client encoding