On Mon, Aug 21, 2023 at 4:09 PM Richard Guo <guofengli...@gmail.com> wrote: > > > On Sun, Aug 20, 2023 at 11:48 PM Tom Lane <t...@sss.pgh.pa.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
