Re: not null constraints, again - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: not null constraints, again
Date
Msg-id 202411091229.k6wy32lley63@alvherre.pgsql
Whole thread Raw
In response to Re: not null constraints, again  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: not null constraints, again
List pgsql-hackers
On 2024-Nov-08, Tom Lane wrote:

> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > But we'll see what else the buildfarm has to say now that I pushed it ...
> 
> A lot of the buildfarm is saying
> 
>  adder         | 2024-11-08 13:04:39 | ../pgsql/src/backend/catalog/pg_constraint.c:708:37: warning: comparison is
alwaystrue due to limited range of data type [-Wtype-limits]
 
> 
> which evidently is about this:
> 
>     Assert(colnum > 0 && colnum <= MaxAttrNumber);

Hah.

> The memcpy right before that doesn't seem like project style either.
> Most other places that are doing similar things just cast the
> ARR_DATA_PTR to the right pointer type and dereference it.

Hmm, yeah, that's easily removed,


diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index e953000c01d..043bf7c24dd 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -704,11 +704,7 @@ extractNotNullColumn(HeapTuple constrTup)
         ARR_DIMS(arr)[0] != 1)
         elog(ERROR, "conkey is not a 1-D smallint array");
 
-    memcpy(&colnum, ARR_DATA_PTR(arr), sizeof(AttrNumber));
-    Assert(colnum > 0 && colnum <= MaxAttrNumber);
-
-    if ((Pointer) arr != DatumGetPointer(adatum))
-        pfree(arr);                /* free de-toasted copy, if any */
+    colnum = ((AttrNumber *) ARR_DATA_PTR(arr))[0];
 
     return colnum;
 }


I notice I cargo-culted a "free de-toasted copy", but I think it's
impossible to end up with a toasted datum here, because the column is
guaranteed to have only one element, so not a candidate for toasting.
But also, if we don't free it (in case somebody does an UPDATE to the
catalog with a large array), nothing happens, because memory is going to
be released soon anyway, by the error that results by conkey not being
one element long.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Si quieres ser creativo, aprende el arte de perder el tiempo"



pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: logical replication: restart_lsn can go backwards (and more), seems broken since 9.4
Next
From: Peter Eisentraut
Date:
Subject: Re: Proper object locking for GRANT/REVOKE