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?
