From: Tom Lane <tgl@sss.pgh.pa.us>
To: marko@joh.to
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17832: ERROR: failed to apply nullingrels to a non-Var in
 HEAD
In-reply-to: <17832-d0a8106cdf1b722e@postgresql.org>
References: <17832-d0a8106cdf1b722e@postgresql.org>
Comments: In-reply-to PG Bug reporting form <noreply@postgresql.org>
	message dated "Sat, 11 Mar 2023 18:05:20 +0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1425490.1678640561.1@sss.pgh.pa.us>
Date: Sun, 12 Mar 2023 13:02:41 -0400
Message-ID: <1425491.1678640561@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1425491.1678640561%40sss.pgh.pa.us>
Precedence: bulk

PG Bug reporting form <noreply@postgresql.org> writes:
> Running the following query in HEAD fails with the error in $SUBJECT:
> SELECT 1
> FROM
> (
>     SELECT NULL::text AS AccountName
> ) Actual FULL JOIN (
>     SELECT text 'foo'
>     UNION ALL
>     SELECT text 'bar'
> ) Expected (AccountName)
> ON TRUE
> WHERE Expected.AccountName IS NULL;

Thanks for the report!  This example is breaking an assumption I made
in adjust_appendrel_attrs:

         * If var->varnullingrels isn't empty, and the translation wouldn't be
         * a Var, we have to fail.  One could imagine wrapping the translated
         * expression in a PlaceHolderVar, but that won't work because this is
         * typically used after freezing placeholders.  Fortunately, the case
         * appears unreachable at the moment.

Not so much :-(.  That comment goes on to speculate about
pre-emptively wrapping translations in PlaceHolderVars, which is
doable but seems inefficient (mainly because we'd have to do it
well in advance of knowing whether it's needed).

The reason we have a problem is that the planner is (in effect) trying
to push down the "Expected.AccountName IS NULL" clause to the UNION
ALL children.  I'm pretty sure that when I wrote this code, I expected
that that would not happen, because it's useless: we will not be able
to apply that clause at the scan level, because it can't be applied
until after the outer join is formed.  So maybe the right thing to do
is not push down the clause, which I think could be implemented by
having set_append_rel_size not forcibly translate the parent's entire
joininfo list, but only those clauses not mentioning any of the rel's
nulling_relids.

I'm slightly worried that after we do this, we might find that some
other example still forces us to remove the implementation restriction
in adjust_appendrel_attrs.  However, it seems like trimming child
joininfo lists like this might be a net win anyway, just by reducing
the amount of effort we spend examining those lists later.

			regards, tom lane