From: Tom Lane <tgl@sss.pgh.pa.us>
To: Julien Rouhaud <rjuju123@gmail.com>
cc: thibaut.madelaine@dalibo.com, Robert Haas <robertmhaas@gmail.com>,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15669: Error with unnest in PG 11 (ERROR: 0A000)
In-reply-to: 
 <CAOBaU_ZE6+9YBL8htVC=hvV7Wv=Dd3i_Ze+NzW+ESuWpSsjoOw@mail.gmail.com>
References: <15669-02fb3296cca26203@postgresql.org>
 <19353.1551800347@sss.pgh.pa.us>
 <CAOBaU_YPJj95_9oskbobWxinOmfu3zJrNAnFD77yuzFTb6tmbA@mail.gmail.com>
 <21482.1551806937@sss.pgh.pa.us>
 <CAOBaU_ZE6+9YBL8htVC=hvV7Wv=Dd3i_Ze+NzW+ESuWpSsjoOw@mail.gmail.com>
Comments: In-reply-to Julien Rouhaud <rjuju123@gmail.com>
	message dated "Tue, 05 Mar 2019 19:22:32 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <20476.1551819776.1@sss.pgh.pa.us>
Date: Tue, 05 Mar 2019 16:02:56 -0500
Message-ID: <20477.1551819776@sss.pgh.pa.us>
Precedence: bulk

So actually, there are more things wrong here than I thought.

1. This logic is pretty stupid, if not outright wrong, for partitioned
relations.  It fixes all the existing paths to have new outputs, and then
goes and generates all-new Append paths which it just adds on to the
existing paths.  There isn't any situation where that makes sense; we may
as well just throw away the existing paths and then generate the new ones.
Aside from being wasteful, I wonder whether it's even correct --- if any
of the existing paths have dependencies on the old reltargets of the
children, there'll be trouble, assuming they survive the cost comparisons.

2. If we have a dummy relation, and we stick a ProjectionPath atop the
existing dummy path, it stops looking like a dummy relation, as indeed
noted in the existing comment.  It's possible that nothing after this
point cares, but I would not exactly bet on that --- I think it's more
likely that we just don't have test cases exercising combinations where
there are nontrivial processing steps remaining.

The most obvious fix for #2 is to change IS_DUMMY_REL() so that it
can still return true once we've inserted SRF path steps.  In HEAD
I'd be inclined to do that by backing off the premature optimization of
equating a dummy rel with something that has a dummy path, and instead
adding a separate bool is_dummy_rel flag to RelOptInfo.  That doesn't
seem real safe to back-patch into v11 unfortunately.  To make matters
worse, since IS_DUMMY_REL() is a macro, it could be that this broken
test is actually compiled into some third-party extensions.  There's
probably little we can do about that except hope that those extensions
are not dealing with SRF-with-dummy-input cases.  What I suggest we do
in v11 is change IS_DUMMY_REL() to be a subroutine that drills down
through any ProjectionPaths to see if it can find a dummy path.

(Having done that, possibly apply_scanjoin_target_to_paths could be
simplified --- I think it might not need to have a separate code
path for dummy rels anymore.)

Thoughts?

			regards, tom lane