Horton

Re: On login trigger: take three

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Greg Nancarrow <gregn4422@gmail.com>, Ivan Panchenko <wao@mail.ru>, Teodor Sigaev <teodor@sigaev.ru>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, vignesh C <vignesh21@gmail.com>, Pavel Stehule <pavel.stehule@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>
Date: 2023-03-22T17:54:37Z
Lists: pgsql-hackers 
On Tue, Mar 15, 2022 at 4:52 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> Yeah, that was the previously posted v25 from the author (who adopted it from
> the original author).  I took the liberty to quickly poke at the review
> comments you had left as well as the ones that I had found to try and progress
> the patch.  0001 should really go in it's own thread though to not hide it from
> anyone interested who isn't looking at this thread.

Some comments on 0001:

- In general, I think we should prefer to phrase options in terms of
what is done, rather than what is not done. For instance, the
corresponding GUC for row-level security is row_security={on|off}, not
ignore_row_security.

- I think it's odd that the GUC in question doesn't accept true and
false and our usual synonyms for those values. I suggest that it
should, even if we want to add more possible values later.

- "ignoreing" is mispleled. So is gux-ignore-event-trigger. "Even
triggers" -> "Event triggers".

- Perhaps the documentation for the GUC should mention that the GUC is
not relevant in single-user mode because event triggers don't fire
then anyway.

- "Disable event triggers during the session." isn't a very good
description because there is in theory nothing to prevent this from
being set in postgresql.conf.

Basically, I think 0001 is a good idea -- I'm much more nervous about
0002. I think we should get 0001 polished up and committed.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Commits

  1. Fix some typos in event trigger docs

  2. Use heap_inplace_update() to unset pg_database.dathasloginevt

  3. Remove the flaky check in event_trigger_login regression test

  4. Fix instable 006_login_trigger.pl test

  5. Add support event triggers on authenticated login

  6. Add GUC for temporarily disabling event triggers

  7. Fix typo in reference to __FreeBSD__.

  8. Restore robustness of TAP tests that wait for postmaster restart.

  9. Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.