Message-ID: <06a5ac56-7d9e-ccd8-6566-f076f5139354@gmail.com>
Date: Mon, 21 Nov 2022 14:32:31 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.5.0
Subject: Re: Split index and table statistics into different types of stats
To: Andres Freund <andres@anarazel.de>
Cc: Melanie Plageman <melanieplageman@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <f572abe7-a1bb-e13b-48c7-2ca150546822@gmail.com>
 <20221101003006.3ehvn2prai26rkrf@awork3.anarazel.de>
 <4cc5476f-4360-1bed-c60d-2c21adf7db13@gmail.com>
 <CAAKRu_asj8kSnukxHV764LTcUz5u-Z+iitjqesDB0yxthSm0Qw@mail.gmail.com>
 <06517d27-7a81-3b60-60c4-9a92c6498981@gmail.com>
 <8e8713ed-1663-f39b-effc-bd3e1632039a@gmail.com>
 <20221116201202.3k74ajawyom2c3eq@awork3.anarazel.de>
 <76dece9f-160a-0210-0396-9146c8e4831a@gmail.com>
 <20221120231906.aayfz5jpipqd6hoz@awork3.anarazel.de>
Content-Language: en-US
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
In-Reply-To: <20221120231906.aayfz5jpipqd6hoz@awork3.anarazel.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/06a5ac56-7d9e-ccd8-6566-f076f5139354%40gmail.com>
Precedence: bulk

Hi,

On 11/21/22 12:19 AM, Andres Freund wrote:
> Hi,
> 
> On 2022-11-18 12:18:38 +0100, Drouvot, Bertrand wrote:
>> On 11/16/22 9:12 PM, Andres Freund wrote:
>>> This still leaves a fair bit of boilerplate. ISTM that the function body
>>> really should just be a single line.
>>>
>>> Might even be worth defining the whole function via a macro. Perhaps something like
>>>
>>> PGSTAT_DEFINE_REL_FIELD_ACCESSOR(PGSTAT_KIND_INDEX, pg_stat_get_index, numscans);
>>
>> Thanks for the feedback!
>>
>> Right, what about something like the following?
>>
>> "
>> #define PGSTAT_FETCH_STAT_ENTRY(pgstat_entry_kind, pgstat_fetch_stat_function, relid, stat_name) \
>> 	do { \
>> 		pgstat_entry_kind *entry = pgstat_fetch_stat_function(relid); \
>> 		PG_RETURN_INT64(entry == NULL ? 0 : (int64) (entry->stat_name)); \
>> 	} while (0)
>>
>> Datum
>> pg_stat_get_index_numscans(PG_FUNCTION_ARGS)
>> {
>> 	PGSTAT_FETCH_STAT_ENTRY(PgStat_StatIndEntry, pgstat_fetch_stat_indentry, PG_GETARG_OID(0), numscans);
>> }
>> "
> 
> 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.

> Now merged.

Thanks!

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com