On Wed, 26 Nov 2025 at 20:29, Bernice Southey <bernice.southey@gmail.com> wrote:
>
> Bernice Southey <bernice.southey@gmail.com> wrote:
> > I went through the history and it seemed to me the repeat rewrite was
> > accidental because of the two ways this method can recurse.
> I mean the repeat rewrite of the cteList was accidental, not the
> repeat rewrite of views. I couldn't think why a view would mean a
> cteList needed re-rewriting, but I know very little about view rules.
I agree that the repeat rewrites look accidental. However, I've been
thinking about this some more, and I realised that the patch as
written isn't correct.
I think it is true that RewriteQuery() doesn't need to rewrite
individual CTEs multiple times. However, that doesn't mean that it
doesn't need to process the cteList at all below the top-level. The
problem is that rule actions may add additional CTEs to the list,
which do need to be processed when recursing.
Here's a simple, dumb example:
---
drop table if exists t1, t1a, t1b, t2, t2a;
create table t1 (a int);
create table t1a (a int);
create table t1b (a int);
create rule t1r as on insert to t1 do instead
with x as (insert into t1a values (1) returning *)
insert into t1b select a from x returning *;
create table t2 (a int);
create table t2a (a int);
create rule t2r as on insert to t2 do instead
with xx as (insert into t1 values (1) returning *)
insert into t2a select a from xx returning *;
insert into t2 values (1);
---
Both rules should be triggered, resulting in 1 row in each of t1a,
t1b, and t2a, which is what happens in HEAD, but with the patch, it
fails to fire rule t1r when it recurses, so you get a row in t1
instead.
This could probably be fixed up fairly easily, by tracking how many of
the CTEs in the list have been processed, and only processing the new
ones, when recursing.
On the other hand, perhaps making rewriteTargetListIU() idempotent
would be a more robust solution. I've not looked in detail at what
would be needed for that though.
Regards,
Dean
