Re: Second RewriteQuery complains about first RewriteQuery in edge case

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Bernice Southey <bernice(dot)southey(at)gmail(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Second RewriteQuery complains about first RewriteQuery in edge case
Date: 2025-11-27 12:10:41
Message-ID: CAEZATCUmHKZwefNTbokHoc2NCrixtxZf+_qxafijxNdhJH34Jw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 26 Nov 2025 at 21:57, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

Attachment Content-Type Size
v2-avoid-rewriting-CTEs-more-than-once.patch text/x-patch 4.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2025-11-27 12:13:55 Re: Remove unused function parameters, part 1: contrib
Previous Message Nazir Bilal Yavuz 2025-11-27 12:07:17 Re: Remove unused function parameters, part 1: contrib