Re: Refactoring SysCacheGetAttr to know when attr cannot be NULL

On 28.02.23 21:14, Daniel Gustafsson wrote:
> 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 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.

Yes please!

I have occasionally wondered whether just passing the isnull argument as
NULL would be sufficient, so we don't need a new function.