On Thu, Apr 23, 2026 at 12:11 PM David Rowley <[email protected]> wrote: > Have you followed through on what happens > for CTEs that do DML and RETURNING? I assume that fails on the nearby > rte->relkind == RELKIND_RELATION, but I didn't debug to check.
Right. The CTE's reference in the outer query is RTE_CTE, and the rte->rtekind == RTE_RELATION check ensures SJE never considers it as a candidate. The CTE itself is planned as a separate Query, where the varno != root->parse->resultRelation check rules out its target relation. > The > only place I see all_result_relids being added to, aside from the > initial setting with bms_make_singleton() is for the inheritance > expansion in expand_single_inheritance_child(), which happens after > join removals. For leaf_result_relids, it's similar. Right. And this makes the comment and commit message not accurate, as inheritance children have not been added yet, as that happens later in add_other_rels_to_query(). Fixed in v2. > I think the Asserts should go at the top of the function next to the > other Asserts. Putting them near the top makes it clearer when reading > code. The Asserts will be very close to the function's header comment, > so it's easier to get a picture about what the function supports and > does, plus, it helps ensure we still get the Asserts before any early > returns are taken. Hmm, I considered that, but I chose the current placement because the Asserts are documenting a specific non-action: "we don't touch these two sets, and here is why." That reads more naturally adjacent to the cleanup of all the other structures, rather than at the top where it would turn into a precondition claim. The existing Asserts at the top check input-parameter validity, which is a different kind of check. On the early-returns argument: remove_self_join_rel() has no early returns today, and adding one would mean forgetting to clear some field, so I don't expect that to change. That said, either location is OK, so happy to move them if you feel strongly. > It may also be useful to decorate adjust_relid_set() with pg_nodiscard. Good suggestion. Done in v2. - Richard
v2-0001-Fix-bogus-calls-in-remove_self_join_rel.patch
Description: Binary data
