[PATCH] Simplify ExecWithoutOverlapsNotEmpty by removing unused parameter

Started by zengman14 days ago3 messages
Jump to latest
#1zengman
zengman@halodbtech.com

Hi all,

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.

```
 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

Attachments:

0001-refactor-Simplify-ExecWithoutOverlapsNotEmpty-functi.patchapplication/octet-stream; charset=ISO-8859-1; name=0001-refactor-Simplify-ExecWithoutOverlapsNotEmpty-functi.patchDownload+4-5
#2Zsolt Parragi
zsolt.parragi@percona.com
In reply to: zengman (#1)
Re: [PATCH] Simplify ExecWithoutOverlapsNotEmpty by removing unused parameter

Hello!

Looks good to me!

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

#3Chao Li
li.evan.chao@gmail.com
In reply to: zengman (#1)
Re: [PATCH] Simplify ExecWithoutOverlapsNotEmpty by removing unused parameter

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 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.

```
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 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 ‘\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/