Re: Oversight in reparameterize_path_by_child leading to executor crash

On Mon, Aug 21, 2023 at 4:09 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
>
>
> 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.

This goes back to my question about how do we keep
path_is_reparameterizable_by_child() and
reparameterize_path_by_child() in sync with each other. This makes it
further hard to do so. One idea I have is to use the same function
(reparameterize_path_by_child()) in two modes 1. actual
reparameterization 2. assessing whether reparameterization is
possible. Essentially combing reparameterize_path_by_child() and
path_is_reparameterizable_by_child(). The name of such a function can
be chosen appropriately. The mode will be controlled by a fourth
argument to the function. When assessing a path no translations happen
and no extra memory is allocated.

--
Best Wishes,
Ashutosh Bapat