RE: Bug in query rewriter - hasModifyingCTE not getting set

From: "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>
To: 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Nancarrow <gregn4422(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: Bug in query rewriter - hasModifyingCTE not getting set
Date: 2021-05-20 14:27:28
Message-ID: TYAPR01MB299041D92E7E3FBF52A0257FFE2A9@TYAPR01MB2990.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>
> 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?

Finally, I think I've understood what you meant. Yes, the current code seems to be wrong. rule_action is different from sub_action only when the rule action (the query specified in CREATE RULE) is INSERT SELECT. In that case, rule_action points to the entire INSERT SELECT, while sub_action points to the SELECT part. So, we should add the CTE list and set hasModifyingCTE/hasRecursive flags in rule_action.

> 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);

Yes. In this case, the WITH clause must be put before INSERT.

The attached patch is based on your version. It includes cosmetic changes to use = instead of |= for boolean variable assignments. make check passed. Also, Greg-san's original failed test case succeeded. I confirmed that the hasModifyingCTE of the top-level rewritten query is set to true now by looking at the output of debug_print_rewritten and debug_print_plan.

Regards
Takayuki Tsunakawa

Attachment Content-Type Size
v3-0001-propagate-CTE-property-flags-in-rewriter.patch application/octet-stream 4.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2021-05-20 14:51:46 Re: multi-install PostgresNode fails with older postgres versions
Previous Message Bernd Helmle 2021-05-20 14:21:57 Re: Alias collision in `refresh materialized view concurrently`