On Thu, May 20, 2021 at 2:45 PM Ivan Panchenko <wao@mail.ru> wrote:
>
> I have upgraded the patch for the 14th version.
>
I have some feedback on the patch:
(1) The patch adds 3 whitespace errors ("git apply <patch-file>"
reports 3 warnings)
(2) doc/src/sgml/catalogs.sgml
CURRENTLY:
This flag is used to avoid extra lookup of pg_event_trigger table on
each backend startup.
SUGGEST:
This flag is used to avoid extra lookups on the pg_event_trigger table
during each backend startup.
(3) doc/src/sgml/config.sgml
CURRENTLY:
Errors in trigger code can prevent user to login to the system.In this
case disabling this parameter in connection string can solve the
problem:
SUGGEST:
Errors in the trigger code can prevent a user from logging in to the
system. In this case, the parameter can be disabled in the connection
string, to allow the user to login:
(4) doc/src/sgml/event-trigger.sgml
(i)
CURRENTLY:
An event trigger fires whenever the event with which it is associated
occurs in the database in which it is defined. Currently, the only
SUGGEST:
An event trigger fires whenever an associated event occurs in the
database in which it is defined. Currently, the only
(ii)
CURRENTLY:
can be useful for client connections logging,
SUGGEST:
can be useful for logging client connections,
(5) src/backend/commands/event_trigger.c
(i) There are two instances of code blocks like:
xxxx = table_open(...);
tuple = SearchSysCacheCopy1(...);
table_close(...);
These should end with: "heap_freetuple(tuple);"
(ii) Typo "nuder" and grammar:
CURRENTLY:
There can be race condition: event trigger may be added after we have
scanned pg_event_trigger table. Repeat this test nuder pg_database
table lock.
SUGGEST:
There can be a race condition: the event trigger may be added after we
have scanned the pg_event_trigger table. Repeat this test under the
pg_database table lock.
(6) src/backend/utils/misc/postgresql.conf.sample
CURRENTLY:
+#enable_client_connection_trigger = true # enables firing the
client_connection trigger when a client connect
SUGGEST:
+#enable_client_connection_trigger = true # enables firing the
client_connection trigger when a client connects
Regards,
Greg Nancarrow
Fujitsu Australia
