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
