Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.300.41.1.7\))
Subject: Re: [PATCH] Simplify ExecWithoutOverlapsNotEmpty by removing unused
 parameter
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <tencent_6AF712884E8D163F7F46073C@qq.com>
Date: Wed, 25 Feb 2026 06:42:58 +0800
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <2422E6F1-685E-4349-AF8A-684C801567B1@gmail.com>
References: <tencent_6AF712884E8D163F7F46073C@qq.com>
To: zengman <zengman@halodbtech.com>
Archived-At: <https://www.postgresql.org/message-id/2422E6F1-685E-4349-AF8A-684C801567B1%40gmail.com>
Precedence: bulk


> On Feb 24, 2026, at 23:09, zengman <zengman@halodbtech.com> wrote:
>=20
> Hi all,
>=20
> I noticed that ExecWithoutOverlapsNotEmpty() accepts an atttypid =
parameter that isn't actually used. The function only needs typtype to =
distinguish between range and multirange types.
> Currently lookup_type_cache() is called just to extract typtype, but I =
think using get_typtype() directly seems more appropriate.
>=20
> ```
> static void ExecWithoutOverlapsNotEmpty(Relation rel, NameData =
attname, Datum attval,
> -                                                                      =
         char typtype, Oid atttypid);
> +                                                                      =
         char typtype);
>=20
> /* ----------------------------------------------------------------
>  *             ExecOpenIndices
> @@ -753,11 +754,10 @@ check_exclusion_or_unique_constraint(Relation =
heap, Relation index,
>                {
>                        TupleDesc       tupdesc =3D =
RelationGetDescr(heap);
>                        Form_pg_attribute att =3D =
TupleDescAttr(tupdesc, attno - 1);
> -                       TypeCacheEntry *typcache =3D =
lookup_type_cache(att->atttypid, 0);
>=20
>                        ExecWithoutOverlapsNotEmpty(heap, att->attname,
>                                                                        =
        values[indnkeyatts - 1],
> -                                                                      =
         typcache->typtype, att->atttypid);
> +                                                                      =
         get_typtype(att->atttypid));
>                }
>        }
>=20
> @@ -1149,7 +1149,7 @@ index_expression_changed_walker(Node *node, =
Bitmapset *allUpdatedCols)
>  * range or multirange in the given attribute.
>  */
> static void
> -ExecWithoutOverlapsNotEmpty(Relation rel, NameData attname, Datum =
attval, char typtype, Oid atttypid)
> +ExecWithoutOverlapsNotEmpty(Relation rel, NameData attname, Datum =
attval, char typtype)
> {
>        bool            isempty;
>        RangeType  *r;
> ```
>=20
> --
> regards,
> Man =
Zeng<0001-refactor-Simplify-ExecWithoutOverlapsNotEmpty-functi.patch>

Removing the parameter atttypid from ExecWithoutOverlapsNotEmpty looks =
okay as it=E2=80=99s a static function and is only called once.

For the other change, I see a difference between lookup_type_cache and =
get_typtype, where lookup_type_cache never returns NULL but =
ereport(ERROR) when oid is invalid; while get_typtype will return =
=E2=80=98\0'. Though ExecWithoutOverlapsNotEmpty() will end up also =
elog(ERROR), the log message is changed.

I am not sure if there could be some edge cases where att->atttypid =
could be invalid. If yes, then this change will lead to a small behavior =
change.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/