Fix rewriter to set hasModifyingCTE correctly on rewritten queries.
commit362e2dcc46195faadd3fa0ba011dd9a8e3829e7a
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 8 Sep 2021 16:05:43 +0000 (8 12:05 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 8 Sep 2021 16:05:47 +0000 (8 12:05 -0400)
tree576dfa6bd53bfa9d3b58b765e0083175fd1f24ab
parentf7c53bb9e309cfea3206f4ed44b38eb2bd1cf726
Fix rewriter to set hasModifyingCTE correctly on rewritten queries.

If we copy data-modifying CTEs from the original query to a replacement
query (from a DO INSTEAD rule), we must set hasModifyingCTE properly
in the replacement query.  Failure to do this can cause various
unpleasantness, such as unsafe usage of parallel plans.  The code also
neglected to propagate hasRecursive, though that's only cosmetic at
the moment.

A difficulty arises if the rule action is an INSERT...SELECT.  We
attach the original query's RTEs and CTEs to the sub-SELECT Query, but
data-modifying CTEs are only allowed to appear in the topmost Query.
For the moment, throw an error in such cases.  It would probably be
possible to avoid this error by attaching the CTEs to the top INSERT
Query instead; but that would require a bunch of new code to adjust
ctelevelsup references.  Given the narrowness of the use-case, and
the need to back-patch this fix, it does not seem worth the trouble
for now.  We can revisit this if we get field complaints.

Per report from Greg Nancarrow.  Back-patch to all supported branches.
(The test case added here does not fail before v10, but there are
plenty of places checking top-level hasModifyingCTE in 9.6, so I have
no doubt that this code change is necessary there too.)

Greg Nancarrow and Tom Lane

Discussion: https://postgr.es/m/CAJcOf-f68DT=26YAMz_i0+Au3TcLO5oiHY5=fL6Sfuits6r+_w@mail.gmail.com
Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com
src/backend/rewrite/rewriteHandler.c
src/test/regress/expected/with.out
src/test/regress/sql/with.sql