Обсуждение: [PATCH] Simplify ExecWithoutOverlapsNotEmpty by removing unused parameter

Hi all,

I noticed that ExecWithoutOverlapsNotEmpty() accepts an atttypid parameter that isn't actually used. The function only
needstyptype 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.

```
 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 heap, Relation index,
                {
                        TupleDesc       tupdesc = RelationGetDescr(heap);
                        Form_pg_attribute att = TupleDescAttr(tupdesc, attno - 1);
-                       TypeCacheEntry *typcache = 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, 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;
```

--
regards,
Man Zeng

Hello!

Looks good to me!

My only comment is that it could use a proper commit message
explaining the changes.

> On Feb 24, 2026, at 23:09, zengman <zengman@halodbtech.com> wrote:
>
> Hi all,
>
> I noticed that ExecWithoutOverlapsNotEmpty() accepts an atttypid parameter that isn't actually used. The function
onlyneeds 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.
>
> ```
> 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 heap, Relation index,
>                {
>                        TupleDesc       tupdesc = RelationGetDescr(heap);
>                        Form_pg_attribute att = TupleDescAttr(tupdesc, attno - 1);
> -                       TypeCacheEntry *typcache = 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, 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;
> ```
>
> --
> regards,
> Man Zeng<0001-refactor-Simplify-ExecWithoutOverlapsNotEmpty-functi.patch>

Removing the parameter atttypid from ExecWithoutOverlapsNotEmpty looks okay as it’s a static function and is only
calledonce. 

For the other change, I see a difference between lookup_type_cache and get_typtype, where lookup_type_cache never
returnsNULL but ereport(ERROR) when oid is invalid; while get_typtype will return ‘\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
leadto a small behavior change. 

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

On Tue, Feb 24, 2026 at 5:43 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> > On Feb 24, 2026, at 23:09, zengman <zengman@halodbtech.com> wrote:
> >
> > Hi all,
> >
> > I noticed that ExecWithoutOverlapsNotEmpty() accepts an atttypid parameter that isn't actually used. The function
onlyneeds 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
moreappropriate. 
> >
> > ```
> > 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 heap, Relation index,
> >                {
> >                        TupleDesc       tupdesc = RelationGetDescr(heap);
> >                        Form_pg_attribute att = TupleDescAttr(tupdesc, attno - 1);
> > -                       TypeCacheEntry *typcache = 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, 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;
> > ```
> >
> > --
> > regards,
> > Man Zeng<0001-refactor-Simplify-ExecWithoutOverlapsNotEmpty-functi.patch>
>
> Removing the parameter atttypid from ExecWithoutOverlapsNotEmpty looks okay as it’s a static function and is only
calledonce. 
>
> For the other change, I see a difference between lookup_type_cache and get_typtype, where lookup_type_cache never
returnsNULL but ereport(ERROR) when oid is invalid; while get_typtype will return ‘\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
leadto 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