Re: Re[3]: On login trigger: take three

On Thu, May 20, 2021 at 2:45 PM Ivan Panchenko <wao(at)mail(dot)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