Re: Oversight in reparameterize_path_by_child leading to executor crash

On Sun, Aug 20, 2023 at 11:48 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I looked over the v3 patch. I think it's going in generally
> the right direction, but there is a huge oversight:
> path_is_reparameterizable_by_child does not come close to modeling
> the success/failure conditions of reparameterize_path_by_child.
> In all the cases where reparameterize_path_by_child can recurse,
> path_is_reparameterizable_by_child has to do so also, to check
> whether the child path is reparameterizable. (I'm somewhat
> disturbed that apparently we have no test cases that caught that
> oversight; can we add one cheaply?)

Thanks for pointing this out. It's my oversight. We have to check the
child path(s) recursively to tell if a path is reparameterizable or not.
I've fixed this in v4 patch, along with a test case. In the test case
we have a MemoizePath whose subpath is TidPath, which is not
reparameterizable. This test case would trigger Assert in v3 patch.

> BTW, I also note from the cfbot that 9e9931d2b broke this patch by
> adding more ADJUST_CHILD_ATTRS calls that need to be modified.
> I wonder if we could get away with having that macro cast to "void *"
> to avoid needing to change all its call sites. I'm not sure whether
> pickier compilers might warn about doing it that way.

Agreed and have done that in v4 patch.

Thanks
Richard