From: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
---|---|
To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
Cc: | 'Katsuragi Yuta' <katsuragiy(at)oss(dot)nttdata(dot)com>, 'vignesh C' <vignesh21(at)gmail(dot)com>, 'Ted Yu' <yuzhihong(at)gmail(dot)com>, 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "'pgsql-hackers(at)lists(dot)postgresql(dot)org'" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, 'Önder Kalacı' <onderkalaci(at)gmail(dot)com>, "'Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com'" <Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com>, 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>, 'Shubham Khanna' <khannashubham1197(at)gmail(dot)com> |
Subject: | Re: [Proposal] Add foreign-server health checks infrastructure |
Date: | 2024-07-26 05:09:26 |
Message-ID: | 768032ee-fb57-494b-b674-1ccb65b6f969@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2024/07/26 12:15, Hayato Kuroda (Fujitsu) wrote:
> Agreed to add the table. I ran a proofreading tool, and it said below points.
> You can revise if they are acceptable.
Yes, I'm okay with these changes. Thanks for the review!
>> I’ve also made several code improvements, for example adding a typedef for
>> pgfdwVersion to simplify its usage, and updated typedefs.list.
>>
>> +enum pgfdwVersion
>> +{
>> + PGFDW_V1_0 = 0,
>> + PGFDW_V1_2,
>> +} pgfdwVersion;
>
> It was intentionally not added, because while developing pg_createsubscriber,
> I got a comment that local-use data have not have to be typedef'd [1]:
I didn't know about the rule regarding local-use enums without typedef,
as there are examples like pgssVersion and pgssStoreKind in pg_stat_statements.c.
However, I guess that keeping typedefs.list small is better.
So, I'm fine with removing the typedef from that enum definition
and updating typedefs.list accordingly.
> ```
> +Datum
> +postgres_fdw_get_connections_1_2(PG_FUNCTION_ARGS)
> +{
> + postgres_fdw_get_connections_internal(fcinfo, PGFDW_V1_2);
> +
> + return (Datum) 0;
> +}
> ```
>
> I know they have a same meaning, but can you clarify why (Datun) 0
> is returned instead of PG_RETURN_VOID()?
You're right. I made the change for readability, as similar
functions in pg_stat_statements are implemented that way.
However, it's not essential. I'm okay with reverting it.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From | Date | Subject | |
---|---|---|---|
Next Message | Laurenz Albe | 2024-07-26 05:09:29 | Re: Incremental backup from a streaming replication standby fails |
Previous Message | Tom Lane | 2024-07-26 04:35:25 | Re: How to check if issue is solved? |