MIME-Version: 1.0
References: <tencent_6AF712884E8D163F7F46073C@qq.com>
 <2422E6F1-685E-4349-AF8A-684C801567B1@gmail.com>
In-Reply-To: <2422E6F1-685E-4349-AF8A-684C801567B1@gmail.com>
From: shihao zhong <zhong950419@gmail.com>
Date: Wed, 11 Mar 2026 12:50:14 -0400
Message-ID: 
 <CAGRkXqR0AXy6pRqp2D0D6h8rjoOZ7xm8mKWXfpDk4zwrqxwCCg@mail.gmail.com>
Subject: Re: [PATCH] Simplify ExecWithoutOverlapsNotEmpty by removing unused
 parameter
To: Chao Li <li.evan.chao@gmail.com>
Cc: zengman <zengman@halodbtech.com>,
	pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAGRkXqR0AXy6pRqp2D0D6h8rjoOZ7xm8mKWXfpDk4zwrqxwCCg%40mail.gmail.com>
Precedence: bulk

On Tue, Feb 24, 2026 at 5:43=E2=80=AFPM Chao Li <li.evan.chao@gmail.com> wr=
ote:
>
>
>
> > On Feb 24, 2026, at 23:09, zengman <zengman@halodbtech.com> wrote:
> >
> > Hi all,
> >
> > I noticed that ExecWithoutOverlapsNotEmpty() accepts an atttypid parame=
ter that isn't actually used. The function only needs typtype to distinguis=
h 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.
> >
> > ```
> > static void ExecWithoutOverlapsNotEmpty(Relation rel, NameData attname,=
 Datum attval,
> > -                                                                      =
         char typtype, Oid atttypid);
> > +                                                                      =
         char typtype);
> >
> > /* ----------------------------------------------------------------
> >  *             ExecOpenIndices
> > @@ -753,11 +754,10 @@ check_exclusion_or_unique_constraint(Relation hea=
p, Relation index,
> >                {
> >                        TupleDesc       tupdesc =3D RelationGetDescr(hea=
p);
> >                        Form_pg_attribute att =3D TupleDescAttr(tupdesc,=
 attno - 1);
> > -                       TypeCacheEntry *typcache =3D lookup_type_cache(=
att->atttypid, 0);
> >
> >                        ExecWithoutOverlapsNotEmpty(heap, att->attname,
> >                                                                        =
        values[indnkeyatts - 1],
> > -                                                                      =
         typcache->typtype, att->atttypid);
> > +                                                                      =
         get_typtype(att->atttypid));
> >                }
> >        }
> >
> > @@ -1149,7 +1149,7 @@ index_expression_changed_walker(Node *node, Bitma=
pset *allUpdatedCols)
> >  * range or multirange in the given attribute.
> >  */
> > static void
> > -ExecWithoutOverlapsNotEmpty(Relation rel, NameData attname, Datum attv=
al, char typtype, Oid atttypid)
> > +ExecWithoutOverlapsNotEmpty(Relation rel, NameData attname, Datum attv=
al, char typtype)
> > {
> >        bool            isempty;
> >        RangeType  *r;
> > ```
> >
> > --
> > regards,
> > Man Zeng<0001-refactor-Simplify-ExecWithoutOverlapsNotEmpty-functi.patc=
h>
>
> Removing the parameter atttypid from ExecWithoutOverlapsNotEmpty looks ok=
ay 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 ge=
t_typtype, where lookup_type_cache never returns NULL but ereport(ERROR) wh=
en oid is invalid; while get_typtype will return =E2=80=98\0'. Though ExecW=
ithoutOverlapsNotEmpty() will end up also elog(ERROR), the log message is c=
hanged.
>
> 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/
>
>
>
>
>
>

That patch looks good to me.

Thanks,
Shihao