Re: BUG #15669: Error with unnest in PG 11 (ERROR: 0A000)

On Wed, Mar 6, 2019 at 7:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Julien Rouhaud <rjuju123@gmail.com> writes:
> > I agree that the RelOptInfo new flag / IS_DUMMY_REL change solution is
> > the best solution.  Let's hope there won't be too many extensions
> > relying on the old IS_DUMMY_REL() macro.
>
> I concluded that that actually doesn't work very well.  The reason that
> things are set up the way they are, I now remember, is that if we exclude
> all children of an appendrel then what we get is a childless Append path.

Yes, I also realized that on my first try with this approach.

> With the current data structure, that means the appendrel is automatically
> recognized as dummy.  If we have a separate flag then we'd have to fix a
> fair number of places to also set the flag.  I'm afraid we'd miss some,
> or even more likely that there'd be extensions that would be silently
> broken because that doesn't work the same anymore.

I was wondering if we couldn't transform IS_DUMMY_REL to something like

 #define IS_DUMMY_REL(r) \
-   ((r)->cheapest_total_path != NULL && \
-    IS_DUMMY_PATH((r)->cheapest_total_path))
+   ((r)->is_dummy_rel == true || ((r)->cheapest_total_path != NULL && \
+    IS_DUMMY_PATH((r)->cheapest_total_path)))

This way, any empty AppendRel would still be recognized as a dummy rel
until a projection is added.  We would simply have to fix
create_projection_path or any function adding a projection to first
explicitly set the rel->is_dummy_rel flag if it's an empty AppendRel.
We shouldn't miss any spot and it should also work with extensions?