From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Japin Li <japinli@hotmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: turn fastgetattr and heap_getattr to inline functions
Date: Thu, 24 Mar 2022 15:32:49 +0100
Message-ID: <202203241432.mt44osd7kkyb@alvherre.pgsql> (raw)
In-Reply-To: <MEYP282MB166986146989BA9DC1F311B0B6199@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
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/
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, japinli@hotmail.com, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org
Subject: Re: turn fastgetattr and heap_getattr to inline functions
In-Reply-To: <202203241432.mt44osd7kkyb@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox