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 |
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 |