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

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Robert Haas <robertmhaas(at)gmail(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 06:47:59
Message-ID: CAFjFpRcDcET8bC8rGz=FX63Ymj6T64GMxrGYx6m100JGCh2A5w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

So, we should not be schema-qualifying types while casting. From deparse.c
/*
* Convert type OID + typmod info into a type name we can ship to the remote
* server. Someplace else had better have verified that this type name is
* expected to be known on the remote end.
*
* This is almost just format_type_with_typemod(), except that if left to
its
* own devices, that function will make schema-qualification decisions based
* on the local search_path, which is wrong. We must schema-qualify all
* type names that are not in pg_catalog. We assume here that built-in
types
* are all in pg_catalog and need not be qualified; otherwise, qualify.
*/
static char *
deparse_type_name(Oid type_oid, int32 typemod)
{
if (is_builtin(type_oid))
return format_type_with_typemod(type_oid, typemod);
else
return format_type_with_typemod_qualified(type_oid, typemod);
}

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2016-06-27 09:10:11 Re: Cleanup in contrib/postgres_fdw/deparse.c
Previous Message Amit Kapila 2016-06-27 05:18:26 Re: Rename max_parallel_degree?