Greg Nancarrow <gregn4...@gmail.com> writes: > On Sun, Feb 7, 2021 at 10:03 AM Tom Lane <t...@sss.pgh.pa.us> wrote: >> I think either the bit about rule_action is unnecessary, or most of >> the code immediately above this is wrong, because it's only updating >> flags in sub_action. Why do you think it's necessary to change >> rule_action in addition to sub_action?
> I believe that the bit about rule_action IS necessary, as it's needed > for the case of INSERT...SELECT, so that hasModifyingCTE is set on the > rewritten INSERT (see comment above the call to > getInsertSelectQuery(), and the "KLUDGE ALERT" comment within that > function). Hm. So after looking at this more, the problem is that the rewrite is producing something equivalent to INSERT INTO bug6051_2 (WITH t1 AS (DELETE FROM bug6051 RETURNING *) SELECT * FROM t1); If you try to do that directly, the parser will give you the raspberry: ERROR: WITH clause containing a data-modifying statement must be at the top level LINE 2: (WITH t1 AS (DELETE FROM bug6051 RETURNING *) SELECT * FROM ... ^ The code throwing that error, in analyzeCTE(), explains /* * We disallow data-modifying WITH except at the top level of a query, * because it's not clear when such a modification should be executed. */ That semantic issue doesn't get any less pressing just because the query was generated by rewrite. So I now think that what we have to do is throw an error if we have a modifying CTE and sub_action is different from rule_action. Not quite sure how to phrase the error though. In view of this, maybe the right thing is to disallow modifying CTEs in rule actions in the first place. I see we already do that for views (i.e. ON SELECT rules), but they're not really any safer in other types of rules. Given that non-SELECT rules are an undertested legacy thing, I'm not that excited about moving mountains to make this case possible. Anyway, I think I'm going to go revert the patch I crammed in last night. There's more here than meets the eye, and right before a release is no time to be fooling with an issue that's been there for years. regards, tom lane