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

On Tue, Jun 21, 2016 at 4:36 PM, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp
> wrote:

> On 2016/06/21 16:27, Rushabh Lathia wrote:
> > Now I was under impression the IS NOT NULL should be always in inverse of
> > IS NULL, but clearly here its not the case with wholerow. But further
> > looking at
> > the document its saying different thing for wholerow:
> >
> > https://www.postgresql.org/docs/9.5/static/functions-comparison.html
> >
> > Note: If the expression is row-valued, then IS NULL is true when the row
> > expression
> > itself is null or when all the row's fields are null, while IS NOT NULL
> is
> > true
> > when the row expression itself is non-null and all the row's fields are
> > non-null.
> > Because of this behavior, IS NULL and IS NOT NULL do not always return
> > inverse
> > results for row-valued expressions, i.e., a row-valued expression that
> > contains
> > both NULL and non-null values will return false for both tests. This
> > definition
> > conforms to the SQL standard, and is a change from the inconsistent
> behavior
> > exhibited by PostgreSQL versions prior to 8.2.
> >
> >
> > And as above documentation clearly says that IS NULL and IS NOT NULL do
> not
> > always return inverse results for row-valued expressions. So need to
> change
> > the
> > deparse logic into postgres_fdw - how ? May be to use IS NULL rather
> then IS
> > NOT NULL?
> >
> > Input/thought?
>
> Perhaps - NOT expr IS NULL? Like in the attached.
>
>
As the documentation describes row is NULL is going to return true when all
the columns in a row are NULL, even though row itself is not null. So, with
your patch a row (null, null, null) is going to be output as a NULL row. Is
that right?

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.
postgres=# select (null, null, null)::text is not null;
?column?
----------
t
(1 row)

postgres=# select null::record::text is not null;
?column?
----------
f
(1 row)

In find_coercion_pathway(), we resort to IO coercion for record::text
explicit coercion. This should always work the way we want as record_out is
a strict function and for non-null values it outputs at least the
parenthesis which will be treated as non-null text.
2253 /*
2254 * If we still haven't found a possibility, consider automatic
casting
2255 * using I/O functions. We allow assignment casts to string
types and
2256 * explicit casts from string types to be handled this way.
(The
2257 * CoerceViaIO mechanism is a lot more general than that, but
this is
2258 * all we want to allow in the absence of a pg_cast entry.) It
would
2259 * probably be better to insist on explicit casts in both
directions,
2260 * but this is a compromise to preserve something of the
pre-8.3
2261 * behavior that many types had implicit (yipes!) casts to
text.
2262 */

PFA the patch with the cast to text. This is probably uglier than expected,
but I don't know any better test to find nullness of a record, the way we
want here. The patch also includes the expected output changes in the
EXPLAIN output.

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