Re: Second RewriteQuery complains about first RewriteQuery in edge case

On Wed, 26 Nov 2025 at 13:34, Bernice Southey <bernice(dot)southey(at)gmail(dot)com> wrote:
>
> Hi,

Hi!

> I get an odd error if a CTE inserts a GENERATED ALWAYS AS IDENTITY
> column, and then tries to modify an automatically updatable view.
>
> create table t(i int generated always as identity);
> create table base(j int);
> create view v as select * from base;
>
> with cte as (insert into t default values returning i)
> delete from v using cte where j = i;
> ERROR: cannot insert a non-DEFAULT value into column "i" Column "i" is
> an identity column defined as GENERATED ALWAYS.
>
> After much digging and thinking, I discovered it's because
> RewriteQuery is processed twice on the CTE, because of the updatable
> view recursion. The first time, the target entry is added. The second
> time, it errors because it's already added.
> cte parse tree: ... {TARGETENTRY :expr {NEXTVALUEEXPR: seqid ...
> In rewriteTargetListIU:
> apply_default = ((new_tle == NULL && commandType == CMD_INSERT) || ...
> if (att_tup->attidentity == ATTRIBUTE_IDENTITY_ALWAYS && !apply_default) ...
> ereport(ERROR, (errcode(ERRCODE_GENERATED_ALWAYS)

Nice catch!

> I wondered, can anything new be added by a re-rewrite of the same
> cteList? Nothing in what I could follow of the subsequent RewriteQuery
> code makes me believe so. This patch is premised on this belief, and
> my above investigation. I may very well be mistaken, given my
> inexperience and gaping knowledge gaps.

I think we have a bug worth fixing here. However, I am not sure about
the approach. For me, a safer option would be
to disallow queries which try to rewrite something for a second time...

> * As a first-time patch submitter, and first-time much else here, I
> fully expect to be flamed for stupidity. This is a crazy function for
> me to touch.
> * I deliberately didn't indent the if-block in the patch, because the
> tab diffs add 140 superfluous interleaved lines of unreadable mess.

IMHO we can make a refactoring patch here and move this big chunk of
logic to separate functions... But this is a side-quest.

> * I was uncertain where to add my new test, and if there's more tests
> that should be added.

Added test are good, but two things:
1) Why with.sql and not generated.sql ?
2) Why do you create table and view as temp relations? The bug have
nothing to do with the temporality of objects?

> I've learned so much from attempting this, that it was worth it just for that.

Sure

--
Best regards,
Kirill Reshke