Subject: inconsistent use of SearchSysCacheCopy

From: Jeff Davis <[email-masked]>
Date: Fri, 06 Dec 2024 16:31:11 -0800

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