On Thu, May 20, 2021 at 2:45 PM Ivan Panchenko <w...@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