event trigger support for PL/Python

Hi

it is registered in commitfest app?

The patch looks well

Regards

Pavel

st 16. 7. 2025 v 14:02 odesílatel Euler Taveira <euler@eulerto.com> napsal:

On Sun, Jul 20, 2025, at 3:07 AM, Pavel Stehule wrote:

it is registered in commitfest app?

The patch looks well

Ops. I forgot to create one. I registered it now.

https://commitfest.postgresql.org/patch/5939/

--
Euler Taveira
EDB https://www.enterprisedb.com/

út 29. 7. 2025 v 14:53 odesílatel Euler Taveira <euler@eulerto.com> napsal:

On Sun, Jul 20, 2025, at 3:07 AM, Pavel Stehule wrote:

it is registered in commitfest app?

The patch looks well

Ops. I forgot to create one. I registered it now.

https://commitfest.postgresql.org/patch/5939/

Thank you

Pavel

Hi

út 29. 7. 2025 v 14:53 odesílatel Euler Taveira <euler@eulerto.com> napsal:

On Sun, Jul 20, 2025, at 3:07 AM, Pavel Stehule wrote:

it is registered in commitfest app?

The patch looks well

Ops. I forgot to create one. I registered it now.

https://commitfest.postgresql.org/patch/5939/

I am checking the code, and I don't like too much an introduction
of PLPyTrigType - more when it is used in
the pair with variable is_trigger. This combination looks strange and it is
a little bit difficult to read for me.

Maybe I prefer some like

typedef enum {
PLPY_CALLED_AS_TRIGGER,
PLPY_CALLED_AS_EVENT_TRIGGER,
PLPY_CALLED_AS_FUNCTION
} PLPyCallType;

and then instead

if (is_trigger == PLPY_NOT_TRIGGER)

the code can looks like

if (call_type == PLPY_CALLED_AS_FUNCTION)
{

}

What do you think?

Regards

Pavel

On Wed, Aug 6, 2025, at 5:16 PM, Pavel Stehule wrote:

I am checking the code, and I don't like too much an introduction of
PLPyTrigType - more when it is used in
the pair with variable is_trigger. This combination looks strange and
it is a little bit difficult to read for me.

Maybe I prefer some like

typedef enum {
PLPY_CALLED_AS_TRIGGER,
PLPY_CALLED_AS_EVENT_TRIGGER,
PLPY_CALLED_AS_FUNCTION
} PLPyCallType;

I used the same pattern as PL/pgSQL

typedef enum PLpgSQL_trigtype
{
PLPGSQL_DML_TRIGGER,
PLPGSQL_EVENT_TRIGGER,
PLPGSQL_NOT_TRIGGER,
} PLpgSQL_trigtype;

Are you suggesting that we should modify it too?

and then instead

if (is_trigger == PLPY_NOT_TRIGGER)

the code can looks like

if (call_type == PLPY_CALLED_AS_FUNCTION)
{

}

The is_trigger variable is similar to fn_is_trigger in PL/pgSQL (see
PLpgSQL_function). If this variable is not clear maybe a prefix should avoid
confusion.

--
Euler Taveira
EDB https://www.enterprisedb.com/

čt 7. 8. 2025 v 2:35 odesílatel Euler Taveira <euler@eulerto.com> napsal:

On Wed, Aug 6, 2025, at 5:16 PM, Pavel Stehule wrote:

I am checking the code, and I don't like too much an introduction of
PLPyTrigType - more when it is used in
the pair with variable is_trigger. This combination looks strange and
it is a little bit difficult to read for me.

Maybe I prefer some like

typedef enum {
PLPY_CALLED_AS_TRIGGER,
PLPY_CALLED_AS_EVENT_TRIGGER,
PLPY_CALLED_AS_FUNCTION
} PLPyCallType;

I used the same pattern as PL/pgSQL

I see it

typedef enum PLpgSQL_trigtype
{
PLPGSQL_DML_TRIGGER,
PLPGSQL_EVENT_TRIGGER,
PLPGSQL_NOT_TRIGGER,
} PLpgSQL_trigtype;

Are you suggesting that we should modify it too?

I am not happy about it, but maybe not, and it is a different issue. I
think fn_is_trigger is a little bit of a messy name too. It worked when
there was no other type of trigger and this variable was boolean. But the
rename breaks plpgsql plugins :-/, and I am not sure if it is an acceptable
cost. Although there are probably four plpgsql plugins, and the change can
be minimal and easy (and well detected)

Minimally - you should to use PLPY_DML_TRIGGER instead PLPY_TRIGGER

and then instead

if (is_trigger == PLPY_NOT_TRIGGER)

the code can looks like

if (call_type == PLPY_CALLED_AS_FUNCTION)
{

}

The is_trigger variable is similar to fn_is_trigger in PL/pgSQL (see
PLpgSQL_function). If this variable is not clear maybe a prefix should
avoid
confusion.

Maybe the name "trigtype" can be better than "is_trigger". The similarity
with PLpgSQL has some benefits, but in this case I think so the plpgsql
design (of this case) is minimally confusing (and really the related part
in plpgsql_compile_callback can be cleaned). How much - this is a question.
There are two different things that are mixed together (and this is what I
dislike):

a) how the function was defined - RETURNS trigger, RETURNS event_trigger,
or something else

b) how the function was called - as dml trigger, as event trigger, or as
function or procedure

You can see, the event trigger has minimal intersection with the dml
trigger - but using the name "is_trigger" or "fn_is_trigger" implies
stronger relations between dml triggers and event triggers.

What do you think?

Regards

Pavel

On Thu, Aug 7, 2025, at 1:53 AM, Pavel Stehule wrote:

Minimally - you should to use PLPY_DML_TRIGGER instead PLPY_TRIGGER

I didn't use DML terminology for the same reason Peter said in another thread
[1]: /messages/by-id/1379995202.8103.4.camel@vanquo.pezone.net

Maybe the name "trigtype" can be better than "is_trigger". The
similarity with PLpgSQL has some benefits, but in this case I think so
the plpgsql design (of this case) is minimally confusing (and really
the related part in plpgsql_compile_callback can be cleaned). How much
- this is a question. There are two different things that are mixed
together (and this is what I dislike):

I'm fine with trigger kind or trigger type but I wouldn't like to use DML
trigger.

[1]: /messages/by-id/1379995202.8103.4.camel@vanquo.pezone.net

--
Euler Taveira
EDB https://www.enterprisedb.com/

pá 8. 8. 2025 v 1:31 odesílatel Euler Taveira <euler@eulerto.com> napsal:

On Thu, Aug 7, 2025, at 1:53 AM, Pavel Stehule wrote:

Minimally - you should to use PLPY_DML_TRIGGER instead PLPY_TRIGGER

I didn't use DML terminology for the same reason Peter said in another
thread
[1]; let's *not* introduce a new terminology (DML trigger).

Maybe the name "trigtype" can be better than "is_trigger". The
similarity with PLpgSQL has some benefits, but in this case I think so
the plpgsql design (of this case) is minimally confusing (and really
the related part in plpgsql_compile_callback can be cleaned). How much
- this is a question. There are two different things that are mixed
together (and this is what I dislike):

I'm fine with trigger kind or trigger type but I wouldn't like to use DML
trigger.

[1]
/messages/by-id/1379995202.8103.4.camel@vanquo.pezone.net

ok

Regards

Pavel

On 16.07.25 14:01, Euler Taveira wrote:

There is an old thread [1] that proposed $SUBJECT. That patch was not committed and the thread died. I use the referred patch as base for the attached version. The key differences between them are: documentation, tests, refactor (0001) and a few cleanups.

Committed.

I altered the documentation a bit to have the event triggers description
in its own sect1, next to normal triggers. This is how it's done in the
other PL chapters, so it made sense to keep it similar.

On Thu, Aug 21, 2025, at 4:46 AM, Peter Eisentraut wrote:

On 16.07.25 14:01, Euler Taveira wrote:

There is an old thread [1] that proposed $SUBJECT. That patch was not committed and the thread died. I use the referred patch as base for the attached version. The key differences between them are: documentation, tests, refactor (0001) and a few cleanups.

Committed.

Thanks.

I altered the documentation a bit to have the event triggers description
in its own sect1, next to normal triggers. This is how it's done in the
other PL chapters, so it made sense to keep it similar.

LGTM.

--
Euler Taveira
EDB https://www.enterprisedb.com/