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
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? |