Re: Oversight in reparameterize_path_by_child leading to executor crash
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2023-08-21T12:23:01Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Postpone reparameterization of paths until create_plan().
- b7e2121ab7d6 17.0 landed
-
Apply band-aid fix for an oversight in reparameterize_path_by_child.
- e031995d5c26 14.11 landed
- 7af96a66f43c 13.14 landed
- 62f120203147 16.2 landed
- 2e822a1d62d0 12.18 landed
- 12ec16d11c8b 15.6 landed
On Mon, Aug 21, 2023 at 4:09 PM Richard Guo <guofenglinux@gmail.com> wrote: > > > On Sun, Aug 20, 2023 at 11:48 PM Tom Lane <tgl@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