Robert Haas <robertmh...@gmail.com> writes: > On Fri, Nov 3, 2023 at 2:47 AM Richard Guo <guofengli...@gmail.com> wrote: >> Comment is now added above the Asserts. Thanks for taking an interest >> in this.
> I'd like to suggest rewording this comment a little more. Here's my proposal: > Both of the paths passed to this function are still parameterized by > the topmost parent, because reparameterize_path_by_child() hasn't been > called yet. So, these sanity checks use the parent relids. > What do you think? I don't think I believe this code change, let alone any of the explanations for it. The point of these Asserts is to be sure that we don't form an alleged parameterization set that includes any rels that are included in the new join, because that would be obviously wrong. They do check that as they stand. I'm not sure what invariant the proposed revision would be enforcing. There might be an argument for leaving the existing Asserts in place and *also* checking Assert(!bms_overlap(outer_paramrels, inner_path->parent->top_parent_relids)); Assert(!bms_overlap(inner_paramrels, outer_path->parent->top_parent_relids)); but I'm not quite convinced what the point of that is. regards, tom lane