Re: turn fastgetattr and heap_getattr to inline functions

On 2022-Mar-24, Japin Li wrote:

> I want to know why we do not use the following style?
> 
> +static inline Datum
> +heap_getattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull)
> +{
> +	if (attnum > 0)
> +	{
> +		if (attnum > (int) HeapTupleHeaderGetNatts(tup->t_data))
> +			return getmissingattr(tupleDesc, attnum, isnull);
> +		else
> +			return fastgetattr(tup, attnum, tupleDesc, isnull);
> +	}
> +
> +	return heap_getsysattr(tup, attnum, tupleDesc, isnull);
> +}

That was the first thing I wrote, but I can't get myself to like it.
For this one function the code flow is obvious enough; but if you apply
the same idea to fastgetattr(), the result is not nice at all.

If there are enough votes for doing it this way, I can do that.

I guess we could do something like this instead, which seems somewhat
less bad:

if (attnum <= 0)
	return heap_getsysattr(...)
if (likely(attnum <= HeapTupleHeaderGetNattrs(...)))
	return fastgetattr(...)

return getmissingattr(...)

but I still prefer the one in the v2 patch I posted.

It's annoying that case 0 (InvalidAttrNumber) is not well handled
anywhere.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/