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/
