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.