Feedback-ID: ia2694551:Fastmail
Date: Fri, 14 Mar 2025 09:35:15 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@alvh.no-ip.org>
To: Nikolay Shaplov <dhyan@nataraj.su>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Minor rework of ALTER TABLE SET RelOptions code
Message-ID: <202503140835.a7rr73tvaeic@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1913854.tdWV9SEqCh@thinkpad-pgpro>
Archived-At: 
 <https://www.postgresql.org/message-id/202503140835.a7rr73tvaeic%40alvherre.pgsql>
Precedence: bulk

On 2025-Mar-07, Nikolay Shaplov wrote:

> Patch description is following:
> 
> 1. `isnull` variable is actually needed in a very narrow scope, so              
> it is better to keep it in that scope, not keeping it in mind in while          
> dealing with the rest of the code.                                              
>                                                                                 
> 2. Toast table RelOptions are never altered directly with ALTER command.        
> One should do ATLER to a heap relation and use toast. reloption namespace       
> to address toast's reloption. If you get `ATExecSetRelOptions` called with      
> `RELKIND_TOASTVALUE` relation in the args, something is really wrong. We        
> should throw asserts, errors and whistle as loud as we can.

These are two separate things, I would have suggested to post them as
two separate patches.  And the second one lacked tests, so I added some.
I have pushed both now, thanks.

I can't unfortunately promise to review the larger reloption refactoring
patch for 18.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Java is clearly an example of money oriented programming"  (A. Stepanov)