Refactoring SysCacheGetAttr to know when attr cannot be NULL

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Refactoring SysCacheGetAttr to know when attr cannot be NULL
Date: 2023-02-28 20:14:19
Message-ID: AD76405E-DB45-46B6-941F-17B1EB3A9076@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Today we have two fairly common patterns around extracting an attr from a
cached tuple:

a = SysCacheGetAttr(OID, tuple, Anum_pg_foo_bar, &isnull);
Assert(!isnull);

a = SysCacheGetAttr(OID, tuple, Anum_pg_foo_bar, &isnull);
if (isnull)
elog(ERROR, "..");

The error message in the elog() cases also vary quite a lot. I've been unable
to find much in terms of guidelines for when to use en elog or an Assert, with
the likelyhood of a NULL value seemingly being the guiding principle (but not
in all cases IIUC).

The attached refactoring introduce SysCacheGetAttrNotNull as a wrapper around
SysCacheGetAttr where a NULL value triggers an elog(). This removes a lot of
boilerplate error handling which IMO leads to increased readability as the
error handling *in these cases* don't add much (there are other cases where
checking isnull does a lot of valuable work of course). Personally I much
prefer the error-out automatically style of APIs like how palloc saves a ton of
checking the returned allocation for null, this aims at providing a similar
abstraction.

This will reduce granularity of error messages, and as the patch sits now it
does so a lot since the message is left to work on - I wanted to see if this
was at all seen as a net positive before spending time on that part. I chose
an elog since I as a user would prefer to hit an elog instead of a silent keep
going with an assert, this is of course debateable.

Thoughts?

--
Daniel Gustafsson

Attachment Content-Type Size
0001-Add-SysCacheGetAttrNotNull-for-guarnteed-not-null-at.patch application/octet-stream 69.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2023-02-28 20:26:07 Re: WIN32 pg_import_system_collations
Previous Message Nathan Bossart 2023-02-28 19:57:40 Re: Beautify pg_walinspect docs a bit