Message-ID: <c528aa30-b609-423e-a154-438969c3e79c@iki.fi>
Date: Thu, 23 Apr 2026 21:49:04 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: CheckAttributeType() forgot to recurse into multiranges
To: Chao Li <li.evan.chao@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
References: <93ce56cd-02a6-4db1-8224-c8999372facc@iki.fi>
 <0B5FC478-013D-45D1-876A-AD3FE0C21E7C@gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <0B5FC478-013D-45D1-876A-AD3FE0C21E7C@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/c528aa30-b609-423e-a154-438969c3e79c%40iki.fi>
Precedence: bulk

On 23/04/2026 06:24, Chao Li wrote:
> I traced this patch set, 0002 looks good, but I have a suspicion about 0001.
> 
> ```
> +	else if (att_typtype == TYPTYPE_MULTIRANGE)
> +	{
> +		/*
> +		 * If it's a multirange, recurse to check its plain range type.
> +		 */
> +		CheckAttributeType(attname, get_multirange_range(atttypid),
> +						   get_range_collation(atttypid),
> +						   containing_rowtypes,
> +						   flags);
> +	}
> ```
> 
> Looking at get_range_collation(), it only searches for RANGETYPE, so get_range_collation(atttypid) here will always return InvalidOid. This does not seem to cause a problem, because CheckAttributeType() will recurse into the TYPTYPE_RANGE path, and the collation will be evaluated there.
> 
> But to make the logic clearer, I think we could just pass InvalidOid as the collation OID in the TYPTYPE_MULTIRANGE case. If we really want to pass the actual collation OID here, I think it would need to be done more like this:
> 
> ```
> 	else if (att_typtype == TYPTYPE_MULTIRANGE)
> 	{
> 		Oid multirange_range_typid = get_multirange_range(atttypid);
> 		Oid collation = get_range_collation(multirange_range_typid);
> 		/*
> 		 * If it's a multirange, recurse to check its plain range type.
> 		 */
> 		CheckAttributeType(attname, multirange_range_typid,
> 						   collation,
> 						   containing_rowtypes,
> 						   flags);
> 	}
> ```
Oh, good catch, this is subtle. I'll change it to InvalidOid. As long as 
range types are not collatable, that'll do the right thing. If range 
types were collatable, I'm not sure what the right thing would be. It 
depends on what exactly the collation of a range type would mean.

So, pushed with InvalidOid. Thanks for the review!

- Heikki