Re: On login trigger: take three

On Tue, Jan 23, 2024 at 11:52 PM Alexander Korotkov
<aekorotkov(at)gmail(dot)com> wrote:
> On Tue, Jan 23, 2024 at 8:00 PM Alexander Lakhin <exclusion(at)gmail(dot)com> wrote:
> > Please look at the following query, which triggers an assertion failure on
> > updating the field dathasloginevt for an entry in pg_database:
> > SELECT format('CREATE DATABASE db1 LOCALE_PROVIDER icu ICU_LOCALE en ENCODING utf8
> > ICU_RULES ''' || repeat(' ', 200000) || ''' TEMPLATE template0;')
> > \gexec
> > \c db1 -
> >
> > CREATE FUNCTION on_login_proc() RETURNS event_trigger AS $$
> > BEGIN
> > RAISE NOTICE 'You are welcome!';
> > END;
> > $$ LANGUAGE plpgsql;
> >
> > CREATE EVENT TRIGGER on_login_trigger ON login EXECUTE PROCEDURE on_login_proc();
> > DROP EVENT TRIGGER on_login_trigger;
> >
> > \c
> >
> > \connect: connection to server on socket "/tmp/.s.PGSQL.5432" failed: server closed the connection unexpectedly
> >
> > The stack trace of the assertion failure is:
> > ...
> > #5 0x000055c8699b9b8d in ExceptionalCondition (
> > conditionName=conditionName(at)entry=0x55c869a1f7c0 "HaveRegisteredOrActiveSnapshot()",
> > fileName=fileName(at)entry=0x55c869a1f4c6 "toast_internals.c", lineNumber=lineNumber(at)entry=669) at assert.c:66
> > #6 0x000055c86945df0a in init_toast_snapshot (...) at toast_internals.c:669
> > #7 0x000055c86945dfbe in toast_delete_datum (...) at toast_internals.c:429
> > #8 0x000055c8694fd1da in toast_tuple_cleanup (...) at toast_helper.c:309
> > #9 0x000055c8694b55a1 in heap_toast_insert_or_update (...) at heaptoast.c:333
> > #10 0x000055c8694a8c6c in heap_update (... at heapam.c:3604
> > #11 0x000055c8694a96cb in simple_heap_update (...) at heapam.c:4062
> > #12 0x000055c869555b7b in CatalogTupleUpdate (...) at indexing.c:322
> > #13 0x000055c8695f0725 in EventTriggerOnLogin () at event_trigger.c:957
> > ...
> >
> > Funnily enough, when Tom Lane was wondering, whether pg_database's toast
> > table could pose a risk [1], I came to the conclusion that it's impossible,
> > but that was before the login triggers introduction...
> >
> > [1] https://www.postgresql.org/message-id/1284094.1695479962%40sss.pgh.pa.us
>
> Thank you for reporting. I'm looking into this...
> I wonder if there is a way to avoid toast update given that we don't
> touch the toasted field here.

Usage of heap_inplace_update() seems appropriate for me here. It
avoids trouble with both TOAST and row-level locks. Alexander, could
you please recheck this fixes the problem.

------
Regards,
Alexander Korotkov