Re: Second RewriteQuery complains about first RewriteQuery in edge case

On Wed, 26 Nov 2025 at 21:57, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I was toying with the idea of decoupling rewriting of WITHs from
> the main recursion.  This would look roughly like
>
> 1. Pull out RewriteQuery's first loop into a new function, say
> RewriteQueryCTEs.  Call this from QueryRewrite just before calling
> RewriteQuery.
>
> 2. Also apply it to a rule action's CTE list in fireRules, before
> we merge the original query's CTE list into that list.

Yes, I think that would work, but I think a simpler solution would be
to just have RewriteQuery() track which CTEs it had already rewritten,
which can be done just by passing around a list of CTE names.
Something like the attached.

I think this makes it more obvious how CTEs only get processed once,
and has the advantage of being a smaller, more localised change.

> given the lack of complaints
> to date, maybe a HEAD-only fix is acceptable.

I was thinking that we should try to create a back-patchable fix for
this. If it had been some complex query involving rules, then perhaps
a HEAD-only fix would be OK, but the original reproducer is a pretty
simple combination of more commonly-used features.

> There's an awful lot of unfinished work about CTEs in this module,
> eg the two different random implementation restrictions in the
> same stanza that's combining the CTE lists, or the one at the
> bottom of RewriteQuery.  I wonder if we have the ambition to
> actually do anything about all that.  People tend to see the
> rules system as a deprecated backwater, so maybe not.

Ugh, yeah, those limitations are not great. Something else I noticed
while trying to produce the test case with rules was that you can't
refer to the OLD or NEW pseudo-relations from within a CTE query in a
rule action. That pretty-much makes data-modifying CTEs in rule
actions useless, I think. But I tend to think of rules as a feature
that should be avoided at all costs in any real applications. In my
mind, that just makes them a maintenance burden, and I wouldn't want
to expend any effort trying to improve them, except if that made them
easier to maintain. That said, I know of real applications that do use
them (apparently without problems), so I can imagine we'd get pushback
if we tried to remove support for them.

Regards,
Dean