RE: Bug in query rewriter - hasModifyingCTE not getting set

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Greg Nancarrow <gregn4422(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: Bug in query rewriter - hasModifyingCTE not getting set
Date: 2021-05-20 05:54:27
Message-ID: OS0PR01MB571681E27B5D237F6DA1B273942A9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
> Greg Nancarrow <gregn4422(at)gmail(dot)com> writes:
> > On Sun, Feb 7, 2021 at 10:03 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.

I am +1 for throwing an error if we have a modifying CTE and sub_action is different
from rule_action. As we disallowed data-modifying CTEs which is not at the top level
of a query, it will be safe and consistent to disallow the same case here.

Maybe we can output the message like the following ?
"DO INSTEAD INSERT ... SELECT rules are not supported for INSERT contains data-modifying statements in WITH."

Best regards,
houzj

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-05-20 06:04:51 Re: "ERROR: deadlock detected" when replicating TRUNCATE
Previous Message Andy Fan 2021-05-20 05:21:42 Re: Condition pushdown: why (=) is pushed down into join, but BETWEEN or >= is not?