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
--
Euler Taveira
EDB https://www.enterprisedb.com/
