Thanks for the review! On Sat, Jan 6, 2024 at 2:36 AM Robert Haas <robertmh...@gmail.com> wrote:
> Richard, I think it could be useful to put a better commit message > into the patch file, describing both what problem is being fixed and > what the design of the fix is. I gather that the problem is that we > crash if the query contains a partioningwise join and also $SOMETHING, > and the solution is to move reparameterization to happen at > createplan() time but with a precheck that runs during path > generation. Presumably, that means this is more than a minimal bug > fix, because the bug could be fixed without splitting > can-it-be-reparameterized to reparameterize-it in this way. Probably > that's a performance optimization, so maybe it's worth clarifying > whether that's just an independently good idea or whether it's a part > of making the bug fix not regress performance. Thanks for the suggestion. Attached is an updated patch which is added with a commit message that tries to explain the problem and the fix. I think the macro names in path_is_reparameterizable_by_child could be > better chosen. CHILD_PATH_IS_REPARAMETERIZABLE doesn't convey that the > macro will return from the calling function if not -- it looks like it > just returns a Boolean. Maybe REJECT_IF_PATH_NOT_REPARAMETERIZABLE and > REJECT_IF_PATH_LIST_NOT_REPARAMETERIZABLE or some such. Agreed. > Another question here is whether we really want to back-patch all of > this or whether it might be better to, as Tom proposed previously, > back-patch a more minimal fix and leave the more invasive stuff for > master. Fair point. I think we can back-patch a more minimal fix, as Tom proposed in [1], which disallows the reparameterization if the path contains sampling info that references the outer rel. But I think we need also to disallow the reparameterization of a path if it contains restriction clauses that reference the outer rel, as such paths have been found to cause incorrect results, or planning errors as in [2]. [1] https://www.postgresql.org/message-id/3163033.1692719009%40sss.pgh.pa.us [2] https://www.postgresql.org/message-id/CAMbWs4-CSR4VnZCDep3ReSoHGTA7E%2B3tnjF_LmHcX7yiGrkVfQ%40mail.gmail.com Thanks Richard
v9-0001-Postpone-reparameterization-of-paths-until-when-creating-plans.patch
Description: Binary data
