pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: jian he <jian.universality@gmail.com>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Tender Wang <tndrwang@gmail.com>
Subject: Re: not null constraints, again
Date: Sat, 9 Nov 2024 13:29:12 +0100
Message-ID: <202411091229.k6wy32lley63@alvherre.pgsql> (raw)
In-Reply-To: <3649828.1731083171@sss.pgh.pa.us>

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 always true 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,


inline patch 1: src/backend/catalog/pg_constraint.c (+1/-5 lines) download inline-1.patchdiff --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"

view thread (89+ messages)  latest in thread

Message-ID: <202411091229.k6wy32lley63@alvherre.pgsql>
Permalink:  ../202411091229.k6wy32lley63@alvherre.pgsql/
Also on:    postgresql.org/message-id/202411091229.k6wy32lley63@alvherre.pgsql

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@alvh.no-ip.org, tgl@sss.pgh.pa.us, jian.universality@gmail.com, pgsql-hackers@lists.postgresql.org, tndrwang@gmail.com
  Subject: Re: not null constraints, again
  In-Reply-To: <202411091229.k6wy32lley63@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox