Hi,
On 11/22/22 8:12 AM, Drouvot, Bertrand wrote:
> Hi,
>
> On 11/22/22 7:19 AM, Bharath Rupireddy wrote:
>> On Mon, Nov 21, 2022 at 7:03 PM Drouvot, Bertrand
>> <bertranddrouvot.pg@gmail.com> wrote:
>>>
>>> On 11/21/22 12:19 AM, Andres Freund wrote:
>>>>
>>>> That's better, but still seems like quite a bit of repetition, given the
>>>> number of accessors. I think I like my idea of a macro defining the whole
>>>> function a bit better.
>>>>
>>>
>>> Got it, what about creating another preparatory commit to first introduce something like:
>>>
>>> "
>>> #define PGSTAT_DEFINE_REL_FIELD_ACCESSOR(function_name_prefix, stat_name) \
>>> Datum \
>>> function_name_prefix##_##stat_name(PG_FUNCTION_ARGS) \
>>> { \
>>> Oid relid = PG_GETARG_OID(0); \
>>> int64 result; \
>>> PgStat_StatTabEntry *tabentry; \
>>> if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL) \
>>> result = 0; \
>>> else \
>>> result = (int64) (tabentry->stat_name); \
>>> PG_RETURN_INT64(result); \
>>> } \
>>>
>>> PGSTAT_DEFINE_REL_FIELD_ACCESSOR(pg_stat_get, numscans);
>>>
>>> PGSTAT_DEFINE_REL_FIELD_ACCESSOR(pg_stat_get, tuples_returned);
>>> .
>>> .
>>> .
>>> "
>>>
>>> If that makes sense to you, I'll submit this preparatory patch.
>>
>> I think the macros stitching the function declarations and definitions
>> is a great idea to avoid code duplicacy. We seem to be using that
>> approach already - PG_FUNCTION_INFO_V1, SH_DECLARE, SH_DEFINE and its
>> friends, STEMMER_MODULE and so on. +1 for first applying this
>> principle for existing functions. Looking forward to the patch.
>>
>
> Thanks! Patch proposal submitted in [1].
>
> I'll resume working on the current thread once [1] is committed.
>
> [1]: https://www.postgresql.org/message-id/d547a9bc-76c2-f875-df74-3ad6fd9d6236%40gmail.com
>
As [1] mentioned above has been committed (83a1a1b566), please find attached V5 related to this thread making use of
thenew macros (namely PG_STAT_GET_RELENTRY_INT64 and PG_STAT_GET_RELENTRY_TIMESTAMPTZ).
I switched from using "CppConcat" to using "##", as it looks to me it's easier to read now that we are adding another
concatenationto the game (due to the table/index split).
The (Tab,tab) or (Ind,ind) passed as arguments to the macros look "weird" (I don't have a better idea yet): purpose is
tofollow the naming convention for PgStat_StatTabEntry/PgStat_StatIndEntry and
pgstat_fetch_stat_tabentry/pgstat_fetch_stat_indentry,thoughts?
Looking forward to your feedback,
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
