Re: wrong comments in rewriteTargetListIU - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: wrong comments in rewriteTargetListIU
Date
Msg-id b69de1f4-2d96-4bc9-b671-722add07c704@eisentraut.org
Whole thread Raw
In response to Re: wrong comments in rewriteTargetListIU  (Dean Rasheed <dean.a.rasheed@gmail.com>)
List pgsql-hackers
On 17.06.25 11:31, Dean Rasheed wrote:
> On Tue, 17 Jun 2025 at 06:27, Peter Eisentraut <peter@eisentraut.org> wrote:
>> Jian is right, there are no OVERRIDING clauses for generated columns,
>> and the shown code also doesn't check for them, so the comment doesn't
>> match the code.  I suspect that commit 17958972fe3b slightly confused
>> generated and identity columns in this case.  I suggest to remove the
>> half-sentence "regardless of ...".
> 
> Hmm, to me that comment seems quite clear in the context of the
> surrounding code. It's making the point that OVERRIDING clauses have
> no effect on what can be inserted into generated columns, unlike the
> identity column handling code that immediately precedes it.
> 
> Perhaps there's an alternate wording that would make that meaning more
> explicit, rather than just removing that part of the comment, which
> might leave people wondering why that code block isn't checking
> "override", when the preceding code blocks do.

I think Jian's point is that the OVERRIDING clause was not created to 
affect generated columns, and so mentioning explicitly that it does not 
affect generated columns does not seem meaningful.  You might as well 
mention any other clause there and say it doesn't affect this particular 
code.

However, I see your point that it contrasts with the attidentity code 
just above.

Perhaps a way to resolve this would be to rewrite the comment something 
like:

     /*
      * Although inserting into a GENERATED BY DEFAULT identity column
      * is allowed, apply the default if OVERRIDING USER VALUE is
      * specified.
      */
     if (att_tup->attidentity == ATTRIBUTE_IDENTITY_BY_DEFAULT &&
         override == OVERRIDING_USER_VALUE)
         apply_default = true;

     /*
      * Can only insert DEFAULT into generated columns.  (The OVERRIDING
      * clause does not apply to generated columns, so we don't consider
      * it here.)
      */
     if (att_tup->attgenerated && !apply_default)
     {




pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: Re: Avoid possible dereference null pointer (contrib/postgres_fdw/postgres_fdw.c)
Next
From: D Laaren
Date:
Subject: Re: Timeline switching with partial WAL records can break replica recovery