Message-ID: <8f8d06fbf986437b1526c6ac68a27165cf4deea3.camel@j-davis.com>
Subject: inconsistent use of SearchSysCacheCopy
From: Jeff Davis <pgsql@j-davis.com>
To: pgsql-hackers@postgresql.org
Date: Fri, 06 Dec 2024 16:31:11 -0800
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
MIME-Version: 1.0
Archived-At: <https://www.postgresql.org/message-id/8f8d06fbf986437b1526c6ac68a27165cf4deea3.camel%40j-davis.com>
Precedence: bulk

When modifying catalog contents, some callers use SearchSysCacheN() and
some use SearchSysCacheCopyN().

for instance, these callers use SearchSysCacheN():

  - AggregateCreate()
  - ProcedureCreate()

while these callers that use SearchSysCacheCopyN():

  - OperatorCreate()
  - TypeCreate()

I generally thought that the non-copy variant was preferred if you are
using heap_modify_tuple() or similar, and the latter used when
necessary (e.g. modifying through the Form_pg_foo pointer). Aside from
avoiding a needless copy, the non-copy variant is also a bit safer
because you'll get a WARNING if you forget to ReleaseSysCache().

On the other hand, if you don't use the copy variant, and you actually
do modify it, that would be quite dangerous; so perhaps that's why the
copy variants are used in so many places where it seems unnecessary.

Is there a preferred convention?

Regards,
	Jeff Davis