Triggers with DO functionality

Hi,

This may have already been discussed before, but I can't find any
mention of it. Would it be desirable to add support for triggers that
contain their own anonymous functions (i.e. DO)?

So instead of

CREATE TRIGGER...
EXECUTE PROCEDURE functioname();

you'd have:

CREATE TRIGGER...
DO $$
...
$$;

The purpose being to only have a single statement to set up the
trigger rather than setting up a separate trigger function which will
unlikely be re-used by other triggers... or is this of dubious
benefit?

--
Thom

On Feb 17, 2012, at 5:22 AM, Thom Brown wrote:

> The purpose being to only have a single statement to set up the
> trigger rather than setting up a separate trigger function which will
> unlikely be re-used by other triggers... or is this of dubious
> benefit?

+1, though I imagine it would just give it a generated name and save it anyway, eh?

David

On 02/17/2012 11:29 AM, David E. Wheeler wrote:
> On Feb 17, 2012, at 5:22 AM, Thom Brown wrote:
>
>> The purpose being to only have a single statement to set up the
>> trigger rather than setting up a separate trigger function which will
>> unlikely be re-used by other triggers... or is this of dubious
>> benefit?
> +1, though I imagine it would just give it a generated name and save it anyway, eh?
>

Before we rush into this, let's consider all the wrinkles. For example,
what if you need to change the function? And how would you edit the
function in psql? It might be a bit more involved that it seems at first
glance, although my initial reaction was the same as David's.

cheers

andrew

On 17 February 2012 16:29, David E. Wheeler <david(at)justatheory(dot)com> wrote:
> On Feb 17, 2012, at 5:22 AM, Thom Brown wrote:
>
>> The purpose being to only have a single statement to set up the
>> trigger rather than setting up a separate trigger function which will
>> unlikely be re-used by other triggers... or is this of dubious
>> benefit?
>
> +1, though I imagine it would just give it a generated name and save it anyway, eh?

I had thought about that, yes, but I didn't want to get bogged down in
implementation.

--
Thom

On 17 February 2012 16:43, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
> On 02/17/2012 11:29 AM, David E. Wheeler wrote:
>>
>> On Feb 17, 2012, at 5:22 AM, Thom Brown wrote:
>>
>>> The purpose being to only have a single statement to set up the
>>> trigger rather than setting up a separate trigger function which will
>>> unlikely be re-used by other triggers... or is this of dubious
>>> benefit?
>>
>> +1, though I imagine it would just give it a generated name and save it
>> anyway, eh?
>>
>
>
>
> Before we rush into this, let's consider all the wrinkles. For example, what
> if you need to change the function? And how would you edit the function in
> psql? It might be a bit more involved that it seems at first glance,
> although my initial reaction was the same as David's.

Why not just...

CREATE OR REPLACE TRIGGER my_trigger...

--
Thom

On 02/17/2012 11:46 AM, Thom Brown wrote:
> On 17 February 2012 16:43, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
>>
>> On 02/17/2012 11:29 AM, David E. Wheeler wrote:
>>> On Feb 17, 2012, at 5:22 AM, Thom Brown wrote:
>>>
>>>> The purpose being to only have a single statement to set up the
>>>> trigger rather than setting up a separate trigger function which will
>>>> unlikely be re-used by other triggers... or is this of dubious
>>>> benefit?
>>> +1, though I imagine it would just give it a generated name and save it
>>> anyway, eh?
>>>
>>
>>
>> Before we rush into this, let's consider all the wrinkles. For example, what
>> if you need to change the function? And how would you edit the function in
>> psql? It might be a bit more involved that it seems at first glance,
>> although my initial reaction was the same as David's.
> Why not just...
>
> CREATE OR REPLACE TRIGGER my_trigger...
>

Maybe that would do it. You might also want a \e command for psql to
match it.

cheers

andrew

On fre, 2012-02-17 at 13:22 +0000, Thom Brown wrote:
> So instead of
>
> CREATE TRIGGER...
> EXECUTE PROCEDURE functioname();
>
> you'd have:
>
> CREATE TRIGGER...
> DO $$
> ...
> $$;

I had wished for this many times and was about to propose something
similar.

We might wish to review the SQL standard and other implementations to
make porting triggers a bit easier too.

Also, whatever ALTER functionality functions have would have to be made
available here as well.

On 17 February 2012 17:26, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> On fre, 2012-02-17 at 13:22 +0000, Thom Brown wrote:
>> So instead of
>>
>> CREATE TRIGGER...
>> EXECUTE PROCEDURE functioname();
>>
>> you'd have:
>>
>> CREATE TRIGGER...
>> DO $$
>> ...
>> $$;
>
> I had wished for this many times and was about to propose something
> similar.
>
> We might wish to review the SQL standard and other implementations to
> make porting triggers a bit easier too.

I had looked at how a couple other RDBMS's do it, and there are:

CREATE TRIGGER...
BEGIN
END;

and

CREATE TRIGGER...
AS
BEGIN
END;

And thinking about it, DO is a bit nonsense here, so maybe we'd just
have something like:

CREATE TRIGGER...
AS $$
BEGIN
END;
$$;

i.e. the same as a function.

--
Thom

Thom Brown <thom(at)linux(dot)com> writes:
> And thinking about it, DO is a bit nonsense here, so maybe we'd just
> have something like:
>
> CREATE TRIGGER...
> AS $$
> BEGIN
> END;
> $$;
>
> i.e. the same as a function.

I like that. How do you tell which language the trigger is written in?
I'm not so sure about other function properties (SET, COST, ROWS,
SECURITY DEFINER etc) because applying default and punting users to go
use the full CREATE FUNCTION syntax would be a practical answer here.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On 17 February 2012 20:40, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
> Thom Brown <thom(at)linux(dot)com> writes:
>> And thinking about it, DO is a bit nonsense here, so maybe we'd just
>> have something like:
>>
>> CREATE TRIGGER...
>> AS $$
>> BEGIN
>> END;
>> $$;
>>
>> i.e. the same as a function.
>
> I like that.  How do you tell which language the trigger is written in?

Exactly the same as a function I'd imagine. Just tack LANGUAGE
<language>; at the end.

> I'm not so sure about other function properties (SET, COST, ROWS,
> SECURITY DEFINER etc) because applying default and punting users to go
> use the full CREATE FUNCTION syntax would be a practical answer here.

*shrug* There's also the question about the stability of the trigger's
own in-line function too (i.e. IMMUTABLE, STABLE, VOLATILE).

--
Thom

On 02/17/2012 03:58 PM, Thom Brown wrote:
> On 17 February 2012 20:40, Dimitri Fontaine<dimitri(at)2ndquadrant(dot)fr> wrote:
>> Thom Brown<thom(at)linux(dot)com> writes:
>>> And thinking about it, DO is a bit nonsense here, so maybe we'd just
>>> have something like:
>>>
>>> CREATE TRIGGER...
>>> AS $$
>>> BEGIN
>>> END;
>>> $$;
>>>
>>> i.e. the same as a function.
>> I like that. How do you tell which language the trigger is written in?
> Exactly the same as a function I'd imagine. Just tack LANGUAGE
> <language>; at the end.
>
>> I'm not so sure about other function properties (SET, COST, ROWS,
>> SECURITY DEFINER etc) because applying default and punting users to go
>> use the full CREATE FUNCTION syntax would be a practical answer here.
> *shrug* There's also the question about the stability of the trigger's
> own in-line function too (i.e. IMMUTABLE, STABLE, VOLATILE).
>

This is going to be pretty much a piece of syntactic sugar. Would it
matter that much if the trigger functions made thus are all volatile? If
someone wants the full function feature set they can always use CREATE
FUNCTION first. I think I'm with Dimitri - let's keep it simple.

cheers

andrew

On 17 February 2012 21:07, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
> On 02/17/2012 03:58 PM, Thom Brown wrote:
>>
>> On 17 February 2012 20:40, Dimitri Fontaine<dimitri(at)2ndquadrant(dot)fr>
>>  wrote:
>>>
>>> Thom Brown<thom(at)linux(dot)com>  writes:
>>>>
>>>> And thinking about it, DO is a bit nonsense here, so maybe we'd just
>>>> have something like:
>>>>
>>>> CREATE TRIGGER...
>>>> AS $$
>>>> BEGIN
>>>> END;
>>>> $$;
>>>>
>>>> i.e. the same as a function.
>>>
>>> I like that.  How do you tell which language the trigger is written in?
>>
>> Exactly the same as a function I'd imagine.  Just tack LANGUAGE
>> <language>; at the end.
>>
>>> I'm not so sure about other function properties (SET, COST, ROWS,
>>> SECURITY DEFINER etc) because applying default and punting users to go
>>> use the full CREATE FUNCTION syntax would be a practical answer here.
>>
>> *shrug* There's also the question about the stability of the trigger's
>> own in-line function too (i.e. IMMUTABLE, STABLE, VOLATILE).
>>
>
> This is going to be pretty much a piece of syntactic sugar. Would it matter
> that much if the trigger functions made thus are all volatile? If someone
> wants the full function feature set they can always use CREATE FUNCTION
> first. I think I'm with Dimitri - let's keep it simple.

Yes, always best to start with essential functionality.

--
Thom

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 02/17/2012 03:58 PM, Thom Brown wrote:
>> On 17 February 2012 20:40, Dimitri Fontaine<dimitri(at)2ndquadrant(dot)fr> wrote:
>>> I'm not so sure about other function properties (SET, COST, ROWS,
>>> SECURITY DEFINER etc) because applying default and punting users to go
>>> use the full CREATE FUNCTION syntax would be a practical answer here.

> This is going to be pretty much a piece of syntactic sugar. Would it
> matter that much if the trigger functions made thus are all volatile? If
> someone wants the full function feature set they can always use CREATE
> FUNCTION first. I think I'm with Dimitri - let's keep it simple.

Volatility is a complete no-op for a trigger function anyway, as are
other planner parameters such as cost/rows, because there is no planning
involved in trigger calls.

Of the existing CREATE FUNCTION options, I think only LANGUAGE, SECURITY
DEFINER, and SET are of any possible interest for a trigger function.
And I don't have any problem deeming SET a second-order thing that you
should have to go use CREATE FUNCTION for. But perhaps SECURITY DEFINER
is a common enough need to justify including in this shorthand form.

Has anybody stopped to look at the SQL standard for this? In-line
trigger definitions are actually what they intend, IIRC.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> Has anybody stopped to look at the SQL standard for this? In-line
> trigger definitions are actually what they intend, IIRC.

In which language? Do we need to include PL/PSM to be compliant, and
use that by default? In that case we might want to force people to
spell out LANGUAGE plpgsql when we don't provide for PSM yet, so that we
avoid some backwards compatibility problems down the road.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> Has anybody stopped to look at the SQL standard for this? In-line
>> trigger definitions are actually what they intend, IIRC.

> In which language? Do we need to include PL/PSM to be compliant, and
> use that by default?

Darn if I know. But let's make sure we don't paint ourselves into a
corner such that we couldn't support the standard's syntax sometime
in the future.

> In that case we might want to force people to
> spell out LANGUAGE plpgsql when we don't provide for PSM yet, so that we
> avoid some backwards compatibility problems down the road.

I suspect that we can avoid that as long as the command is based around
a string literal for the function body. OTOH, CREATE FUNCTION has never
had a default for LANGUAGE, and we don't get many complaints about that,
so maybe insisting that LANGUAGE be supplied for an in-line trigger
isn't unreasonable.

regards, tom lane

On Fri, Feb 17, 2012 at 4:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Has anybody stopped to look at the SQL standard for this?  In-line
> trigger definitions are actually what they intend, IIRC.
>

this is what i found there

<trigger definition> ::=
  CREATE TRIGGER <trigger name> <trigger action time> <trigger event>
      ON <table name> [ REFERENCING <transition table or variable list> ]
      <triggered action>

<triggered action> ::=
  [ FOR EACH { ROW | STATEMENT } ]
      [ WHEN <left paren> <search condition> <right paren> ]
      <triggered SQL statement>

<triggered SQL statement> ::=
    <SQL procedure statement>
  | BEGIN ATOMIC { <SQL procedure statement> <semicolon> }... END

--
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

On Fri, Feb 17, 2012 at 11:43:53AM -0500, Andrew Dunstan wrote:
> On 02/17/2012 11:29 AM, David E. Wheeler wrote:
> >On Feb 17, 2012, at 5:22 AM, Thom Brown wrote:
> >>The purpose being to only have a single statement to set up the
> >>trigger rather than setting up a separate trigger function which will
> >>unlikely be re-used by other triggers... or is this of dubious
> >>benefit?
> >+1, though I imagine it would just give it a generated name and save it anyway, eh?
> Before we rush into this, let's consider all the wrinkles. For
> example, what if you need to change the function? And how would you
> edit the function in psql? It might be a bit more involved that it
> seems at first glance, although my initial reaction was the same as
> David's.

Another complication: anonymous triggers would either have to be
alone, or provide a mechanism to manage a sequence of anonymous
triggers on the same table (such as "replace the third trigger with
..." or "move trigger #4 in position #2", or deciding their order of
execution).

Cheers,
Dr. Gianni Ciolli - 2ndQuadrant Italia
PostgreSQL Training, Services and Support
gianni(dot)ciolli(at)2ndquadrant(dot)it | www.2ndquadrant.it

On 23 February 2012 07:15, Gianni Ciolli <gianni(dot)ciolli(at)2ndquadrant(dot)it> wrote:
> On Fri, Feb 17, 2012 at 11:43:53AM -0500, Andrew Dunstan wrote:
>> On 02/17/2012 11:29 AM, David E. Wheeler wrote:
>> >On Feb 17, 2012, at 5:22 AM, Thom Brown wrote:
>> >>The purpose being to only have a single statement to set up the
>> >>trigger rather than setting up a separate trigger function which will
>> >>unlikely be re-used by other triggers... or is this of dubious
>> >>benefit?
>> >+1, though I imagine it would just give it a generated name and save it anyway, eh?
>> Before we rush into this, let's consider all the wrinkles. For
>> example, what if you need to change the function? And how would you
>> edit the function in psql? It might be a bit more involved that it
>> seems at first glance, although my initial reaction was the same as
>> David's.
>
> Another complication: anonymous triggers would either have to be
> alone, or provide a mechanism to manage a sequence of anonymous
> triggers on the same table (such as "replace the third trigger with
> ..." or "move trigger #4 in position #2", or deciding their order of
> execution).

Isn't the order of execution alphabetical by trigger name in
PostgreSQL? The Triggers themselves wouldn't be anonymous, we'd still
be naming them. It's the referenced functions that would no longer
need defining, and even those probably won't technically be anonymous
as they'll need cataloguing somewhere.

--
Thom

On Thu, Feb 23, 2012 at 08:26:47AM +0000, Thom Brown wrote:
> On 23 February 2012 07:15, Gianni Ciolli <gianni(dot)ciolli(at)2ndquadrant(dot)it> wrote:
> > Another complication: anonymous triggers would either have to be
> > alone, or provide a mechanism to manage a sequence of anonymous
> > triggers on the same table (such as "replace the third trigger with
> > ..." or "move trigger #4 in position #2", or deciding their order of
> > execution).
>
> Isn't the order of execution alphabetical by trigger name in
> PostgreSQL? The Triggers themselves wouldn't be anonymous, we'd still
> be naming them. It's the referenced functions that would no longer
> need defining, and even those probably won't technically be anonymous
> as they'll need cataloguing somewhere.

You're right, sorry.

I misread the proposal as "anonymous triggers" when instead it is
"(named) triggers each implemented via an anonymous function".

Cheers,
Dr. Gianni Ciolli - 2ndQuadrant Italia
PostgreSQL Training, Services and Support
gianni(dot)ciolli(at)2ndquadrant(dot)it | www.2ndquadrant.it

On fre, 2012-02-17 at 16:46 -0500, Tom Lane wrote:
> But perhaps SECURITY DEFINER is a common enough need to justify
> including in this shorthand form.

According to the SQL standard, trigger actions run in security definer
mode. I would hope that we could go with that by default for inline
trigger actions, because it's the thing that makes sense for triggers
most of the time anyway, I think.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On fre, 2012-02-17 at 16:46 -0500, Tom Lane wrote:
>> But perhaps SECURITY DEFINER is a common enough need to justify
>> including in this shorthand form.

> According to the SQL standard, trigger actions run in security definer
> mode. I would hope that we could go with that by default for inline
> trigger actions, because it's the thing that makes sense for triggers
> most of the time anyway, I think.

Uh, I'm not sure that we are talking about the same thing. By default,
a trigger function runs as the table owner, ie it's implicitly SEC DEF
to the table owner. Are you saying the spec expects something different
from that?

(Thinks some more...) Actually, the point of SECURITY DEFINER on a
trigger function is to run as somebody other than the table owner,
to wit the function owner. And with an anonymous function there
couldn't be any other owner. So I guess there is no need for this
clause in this context.

regards, tom lane

On fre, 2012-02-24 at 14:27 -0500, Tom Lane wrote:
> (Thinks some more...) Actually, the point of SECURITY DEFINER on a
> trigger function is to run as somebody other than the table owner,
> to wit the function owner. And with an anonymous function there
> couldn't be any other owner. So I guess there is no need for this
> clause in this context.

You're right. The whole clause will be useless in this case.

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> By default, a trigger function runs as the table owner, ie it's
implicitly SEC DEF
> to the table owner.

Really? That's certainly what I would *want*, but it's not what I've
seen.

test=# create user bob;
CREATE ROLE
test=# create user ted;
CREATE ROLE
test=# alter database test owner to bob;
ALTER DATABASE
test=# set role bob;
SET
test=> create table t (id int not null primary key, val text);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t_pkey"
for table "t"
CREATE TABLE
test=> create table s (id int not null primary key, val text not
null);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "s_pkey"
for table "s"
CREATE TABLE
test=> grant select, insert, update, delete on t to ted;
GRANT
test=> grant select on s to ted;
GRANT
test=> create function t_ins_func() returns trigger language plpgsql as
$$
test$> begin
test$> if new.val is not null then
test$> insert into s (id, val) values (new.id, new.val);
test$> end if;
test$> return new;
test$> end;
test$> $$;
CREATE FUNCTION
test=> create trigger t_ins_trig before insert on t for each row
execute procedure t_ins_func();
CREATE TRIGGER
test=> reset role; set role ted;
RESET
SET
test=> insert into t values (1, null);
INSERT 0 1
test=> select * from s;
id | val
----+-----
(0 rows)

test=> select * from t;
id | val
----+-----
1 |
(1 row)

test=> insert into t values (2, 'two');
ERROR: permission denied for relation s
CONTEXT: SQL statement "insert into s (id, val) values (new.id,
new.val)"
PL/pgSQL function t_ins_func() line 4 at SQL statement

-Kevin

On Fri, Feb 24, 2012 at 2:55 PM, Kevin Grittner
<Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> By default, a trigger function runs as the table owner, ie it's
> implicitly SEC DEF
>> to the table owner.
>
> Really?  That's certainly what I would *want*, but it's not what I've
> seen.

Yeah, not quite consistent with what I've seen.

And it's not obvious that it truly is what you want. An audit trigger
would need to run as the *audit table* owner, which might not be the
same as the user that owns the table on which the trigger fires.
--
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

On fre, 2012-02-24 at 13:55 -0600, Kevin Grittner wrote:
> > By default, a trigger function runs as the table owner, ie it's
> implicitly SEC DEF
> > to the table owner.
>
> Really? That's certainly what I would *want*, but it's not what I've
> seen.

Yes, you're right, that was my recollection as well. I was doubly
confused.

> Kevin Grittner wrote:
>> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> By default, a trigger function runs as the table owner, ie it's
>>> implicitly SEC DEF to the table owner.
>>
>> Really? That's certainly what I would *want*, but it's not what
>> I've seen.
>>
>> [self-contained example of that not happening]

Christopher Browne <cbbrowne(at)gmail(dot)com> wrote:
>
> Yeah, not quite consistent with what I've seen.

Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>
> Yes, you're right

As far as I can tell, triggers run as the user performing the
operation which fires the trigger, not as the owner of the table.

Can anyone provide an example of a trigger running as the table
owner? Is there a bug here? Something for the docs?

Test case (slightly modified) in runnable format, rather than a
copy/paste of a run:

create user bob;
create user ted;
--
set role bob;
create table t (id int not null primary key, val text);
create table s (id int not null primary key, val text not null);
grant select, insert, update, delete on t to ted;
grant select on s to ted;
create function t_ins_func() returns trigger language plpgsql as
$$
begin
raise notice 'role = ''%''', current_user;
if new.val is not null then
insert into s (id, val) values (new.id, new.val);
end if;
return new;
end;
$$;
create trigger t_ins_trig before insert on t
for each row execute procedure t_ins_func();
--
reset role; set role ted;
insert into t values (1, null);
select * from s;
select * from t;
insert into t values (2, 'two');

-Kevin

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> As far as I can tell, triggers run as the user performing the
> operation which fires the trigger, not as the owner of the table.
> Can anyone provide an example of a trigger running as the table
> owner? Is there a bug here? Something for the docs?

A quick look into trigger.c shows that there is no attempt to switch
current userid, so we were clearly all wrong about that. Not sure
why everyone recollected the opposite.

On reflection, there's a fairly clear reason why not to switch userid:
it would break triggers that do something like what's shown in the very
first example in the plpgsql trigger documentation:

-- Remember who changed the payroll when
NEW.last_date := current_timestamp;
NEW.last_user := current_user;
RETURN NEW;

So, whatever the desirability of having them run as table owner,
we can't just up and change that. At minimum we'd need to provide
some function to get at the "calling userid" (or perhaps make that
a new trigger argument?) and have a reasonable grace period for
people to change over to using that.

This might be something to consider in the adjacent thread about command
triggers, too --- who do they run as, and if it's not the calling user,
how do they find out who that is?

regards, tom lane

On Mon, Feb 27, 2012 at 6:20 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
>> As far as I can tell, triggers run as the user performing the
>> operation which fires the trigger, not as the owner of the table.
>  > Can anyone provide an example of a trigger running as the table
>> owner?  Is there a bug here?  Something for the docs?
>
> A quick look into trigger.c shows that there is no attempt to switch
> current userid, so we were clearly all wrong about that.  Not sure
> why everyone recollected the opposite.
>
> On reflection, there's a fairly clear reason why not to switch userid:
> it would break triggers that do something like what's shown in the very
> first example in the plpgsql trigger documentation:
>
>        -- Remember who changed the payroll when
>        NEW.last_date := current_timestamp;
>        NEW.last_user := current_user;
>        RETURN NEW;
>
> So, whatever the desirability of having them run as table owner,
> we can't just up and change that.  At minimum we'd need to provide
> some function to get at the "calling userid" (or perhaps make that
> a new trigger argument?) and have a reasonable grace period for
> people to change over to using that.

I'm inclined to hold to the argument that it Works Properly Now, and
that we shouldn't break it by changing it.

The user *can* be changed, by running a security definer trigger function.

The behaviour that is under consideration seems to be to use something
akin to "security definer as table owner". If someone *wants* that,
then they can readily accomplish that TODAY by altering the function
to make it a SECURITY DEFINER, and change owner to the table owner.

But if we change to have that be the default, it's nowhere near as
easy to unravel it, and to get to the situation where the trigger runs
with the security context of the user that ran the query. SECURITY
DEFINER is more static than that.

> This might be something to consider in the adjacent thread about command
> triggers, too --- who do they run as, and if it's not the calling user,
> how do they find out who that is?

I'm inclined to hold to the same position on that.

- If you *WANT* the command trigger to run as user "frotz", then have
it be a security definer function owned by "frotz."
- Otherwise, it runs with the privileges of the calling user.

That doesn't seem woefully wrong to me.
--
When confronted by a difficult problem, solve it by reducing it to the
question, "How would the Lone Ranger handle this?"

Christopher Browne <cbbrowne(at)gmail(dot)com> writes:
> On Mon, Feb 27, 2012 at 6:20 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> So, whatever the desirability of having them run as table owner,
>> we can't just up and change that.

> I'm inclined to hold to the argument that it Works Properly Now, and
> that we shouldn't break it by changing it.

I would say the same, or at least that any argument for changing it is
probably not strong enough to trump backwards compatibility.

However, Peter seems to think the other way is required by standard.
We can get away with defining whatever behavior we want for triggers
that invoke functions, since that syntax is nonstandard anyway. But,
if you remember the original point of this thread, it was to add syntax
that is pretty nearly equivalent to the spec's. If we're going to do
that, it had better also have semantics similar to the spec's.

So (assuming Peter has read the spec correctly) I'm coming around to the
idea that the anonymous trigger functions created by this syntax ought
to be "SECURITY DEFINER table_owner".

regards, tom lane

Excerpts from Tom Lane's message of lun feb 27 20:49:36 -0300 2012:

> So (assuming Peter has read the spec correctly) I'm coming around to the
> idea that the anonymous trigger functions created by this syntax ought
> to be "SECURITY DEFINER table_owner".

I don't remember all the details, but I had a look at this in the
standard about a year ago and the behavior it mandated wasn't trivially
implemented using our existing mechanism. I mentioned the issue of a
stack of user authorizations that is set up whenever a "routine"
(function) is entered, during last year's PGCon developer's meeting. I
intended to have a look at implementing that, but I haven't done
anything yet. What was clear to me was that once I explained the
problem, everyone seemed to agree that fixing it required more than some
trivial syntax rework.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

2012/2/28 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Christopher Browne <cbbrowne(at)gmail(dot)com> writes:
>> On Mon, Feb 27, 2012 at 6:20 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> So, whatever the desirability of having them run as table owner,
>>> we can't just up and change that.
>
>> I'm inclined to hold to the argument that it Works Properly Now, and
>> that we shouldn't break it by changing it.
>
> I would say the same, or at least that any argument for changing it is
> probably not strong enough to trump backwards compatibility.
>

+1

> However, Peter seems to think the other way is required by standard.
> We can get away with defining whatever behavior we want for triggers
> that invoke functions, since that syntax is nonstandard anyway.  But,
> if you remember the original point of this thread, it was to add syntax
> that is pretty nearly equivalent to the spec's.  If we're going to do
> that, it had better also have semantics similar to the spec's.
>
> So (assuming Peter has read the spec correctly) I'm coming around to the
> idea that the anonymous trigger functions created by this syntax ought
> to be "SECURITY DEFINER table_owner".
>

It should be strange if using two forms of one code can have two
relative different behave.

Actually we are in opposition to spec, because it expect SECURITY
DEFINER for all stored procedures. All logic about rights are
consistent now and I am not for changes in this area.

Regards

Pavel

>                        regards, tom lane
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> This might be something to consider in the adjacent thread about command
> triggers, too --- who do they run as, and if it's not the calling user,
> how do they find out who that is?

As of now, calling user (we just calling a function), or another user if
the function is SECURITY DEFINER. Also, the current patch makes command
triggers superuser only.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On 17 February 2012 22:42, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote:

> On Fri, Feb 17, 2012 at 4:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Has anybody stopped to look at the SQL standard for this? In-line
> > trigger definitions are actually what they intend, IIRC.
> >
>
> this is what i found there
>
> <trigger definition> ::=
> CREATE TRIGGER <trigger name> <trigger action time> <trigger event>
> ON <table name> [ REFERENCING <transition table or variable list> ]
> <triggered action>
>
> <triggered action> ::=
> [ FOR EACH { ROW | STATEMENT } ]
> [ WHEN <left paren> <search condition> <right paren> ]
> <triggered SQL statement>
>
> <triggered SQL statement> ::=
> <SQL procedure statement>
> | BEGIN ATOMIC { <SQL procedure statement> <semicolon> }... END

*slightly delayed response*

So it looks like the standard doesn't complicate the proposal from what I
can tell.

Here's our current syntax:

CREATE [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF } { event
[ OR ... ] }
ON table_name
[ FROM referenced_table_name ]
[ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY
DEFERRED } ]
[ FOR [ EACH ] { ROW | STATEMENT } ]
[ WHEN ( condition ) ]
EXECUTE PROCEDURE function_name ( arguments )

Here's an updated syntax as per the proposal:

CREATE [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF } { event
[ OR ... ] }
ON table_name
[ FROM referenced_table_name ]
[ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY
DEFERRED } ]
[ FOR [ EACH ] { ROW | STATEMENT } ]
[ WHEN ( condition ) ]
{ EXECUTE PROCEDURE function_name ( arguments )
| AS 'trigger function definition' [ LANGUAGE lang_name ]
[ SET configuration_parameter { TO value | = value | FROM CURRENT }
]
}

Example:

CREATE TRIGGER trg_my_trigger
BEFORE INSERT ON customers
FOR EACH ROW
AS $$
BEGIN
IF NEW.status IS NULL THEN
...
END;
$$ LANGUAGE plpgsql SET search_path = shop;

All anonymous trigger functions would be implicitly volatile. I imagine
that the function would need to be "owned" by the trigger, meaning the
function is dropped with the trigger.

So should this then just create a function named after the trigger, perhaps
with a leading underscore? (e.g. _trg_my_trigger)

I would expect that the only differences between this and a regular
trigger-function pair would be:

The function is auto-generated and named after the trigger.
The function has deptype of 'i' (DEPENDENCY_INTERNAL) so that it can only
be dropped as part of the trigger.
The function can't be the target of ALTER FUNCTION, or if it can, only a
relevant sub-set.
The function can't be the target of CREATE OR REPLACE FUNCTION.

And then there are event triggers, which could have the same functionality.

Thom

On 2014-09-16 13:15:59 +0100, Thom Brown wrote:
> On 17 February 2012 22:42, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote:
>
> > On Fri, Feb 17, 2012 at 4:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > >
> > > Has anybody stopped to look at the SQL standard for this? In-line
> > > trigger definitions are actually what they intend, IIRC.
> > >
> >
> > this is what i found there
> >
> > <trigger definition> ::=
> > CREATE TRIGGER <trigger name> <trigger action time> <trigger event>
> > ON <table name> [ REFERENCING <transition table or variable list> ]
> > <triggered action>
> >
> > <triggered action> ::=
> > [ FOR EACH { ROW | STATEMENT } ]
> > [ WHEN <left paren> <search condition> <right paren> ]
> > <triggered SQL statement>
> >
> > <triggered SQL statement> ::=
> > <SQL procedure statement>
> > | BEGIN ATOMIC { <SQL procedure statement> <semicolon> }... END
>
>
> *slightly delayed response*
>
> So it looks like the standard doesn't complicate the proposal from what I
> can tell.
>
> Here's our current syntax:
>
> CREATE [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF } { event
> [ OR ... ] }
> ON table_name
> [ FROM referenced_table_name ]
> [ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY
> DEFERRED } ]
> [ FOR [ EACH ] { ROW | STATEMENT } ]
> [ WHEN ( condition ) ]
> EXECUTE PROCEDURE function_name ( arguments )
>
> Here's an updated syntax as per the proposal:
>
> CREATE [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF } { event
> [ OR ... ] }
> ON table_name
> [ FROM referenced_table_name ]
> [ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY
> DEFERRED } ]
> [ FOR [ EACH ] { ROW | STATEMENT } ]
> [ WHEN ( condition ) ]
> { EXECUTE PROCEDURE function_name ( arguments )
> | AS 'trigger function definition' [ LANGUAGE lang_name ]
> [ SET configuration_parameter { TO value | = value | FROM CURRENT }
> ]
> }

I'm unconvinced that that's sufficient. You already noticed that you
need to add SET here. What's with e.g. SECURITY DEFINER? What's with
AS 'obj_file', 'link_symbol' when you create a C function? I think this
really would need to incorporate a more fundamental subset of CREATE
FUNCTION functionality.

> All anonymous trigger functions would be implicitly volatile. I imagine
> that the function would need to be "owned" by the trigger, meaning the
> function is dropped with the trigger.

Right, that's necessary.

> So should this then just create a function named after the trigger, perhaps
> with a leading underscore? (e.g. _trg_my_trigger)

Hm...

> I would expect that the only differences between this and a regular
> trigger-function pair would be:
>
> The function is auto-generated and named after the trigger.

ok.

> The function has deptype of 'i' (DEPENDENCY_INTERNAL) so that it can only
> be dropped as part of the trigger.

ok.

> The function can't be the target of ALTER FUNCTION, or if it can, only a
> relevant sub-set.

ok.

> The function can't be the target of CREATE OR REPLACE FUNCTION.

That *really* sucks. To the point of making the feature useless in my
eyes. That's really something frequently done.

> And then there are event triggers, which could have the same functionality.

I think the need is much less there. You'll hardly create as many even
triggers as you create triggers on relations. Doesn't seem worth the effort.

Greetings,

Andres Freund

On 16 September 2014 13:29, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:

> On 2014-09-16 13:15:59 +0100, Thom Brown wrote:
> > On 17 February 2012 22:42, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote:
> >
> > > On Fri, Feb 17, 2012 at 4:46 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > > >
> > > > Has anybody stopped to look at the SQL standard for this? In-line
> > > > trigger definitions are actually what they intend, IIRC.
> > > >
> > >
> > > this is what i found there
> > >
> > > <trigger definition> ::=
> > > CREATE TRIGGER <trigger name> <trigger action time> <trigger event>
> > > ON <table name> [ REFERENCING <transition table or variable
> list> ]
> > > <triggered action>
> > >
> > > <triggered action> ::=
> > > [ FOR EACH { ROW | STATEMENT } ]
> > > [ WHEN <left paren> <search condition> <right paren> ]
> > > <triggered SQL statement>
> > >
> > > <triggered SQL statement> ::=
> > > <SQL procedure statement>
> > > | BEGIN ATOMIC { <SQL procedure statement> <semicolon> }... END
> >
> >
> > *slightly delayed response*
> >
> > So it looks like the standard doesn't complicate the proposal from what
> I
> > can tell.
> >
> > Here's our current syntax:
> >
> > CREATE [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF } {
> event
> > [ OR ... ] }
> > ON table_name
> > [ FROM referenced_table_name ]
> > [ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY
> > DEFERRED } ]
> > [ FOR [ EACH ] { ROW | STATEMENT } ]
> > [ WHEN ( condition ) ]
> > EXECUTE PROCEDURE function_name ( arguments )
> >
> > Here's an updated syntax as per the proposal:
> >
> > CREATE [ CONSTRAINT ] TRIGGER name { BEFORE | AFTER | INSTEAD OF } {
> event
> > [ OR ... ] }
> > ON table_name
> > [ FROM referenced_table_name ]
> > [ NOT DEFERRABLE | [ DEFERRABLE ] { INITIALLY IMMEDIATE | INITIALLY
> > DEFERRED } ]
> > [ FOR [ EACH ] { ROW | STATEMENT } ]
> > [ WHEN ( condition ) ]
> > { EXECUTE PROCEDURE function_name ( arguments )
> > | AS 'trigger function definition' [ LANGUAGE lang_name ]
> > [ SET configuration_parameter { TO value | = value | FROM
> CURRENT }
> > ]
> > }
>
> I'm unconvinced that that's sufficient. You already noticed that you
> need to add SET here. What's with e.g. SECURITY DEFINER? What's with
> AS 'obj_file', 'link_symbol' when you create a C function? I think this
> really would need to incorporate a more fundamental subset of CREATE
> FUNCTION functionality.
>

Fair enough, although others have mentioned that SECURITY DEFINER is pretty
much redundant on trigger functions anyway.

> > The function can't be the target of CREATE OR REPLACE FUNCTION.
>
> That *really* sucks. To the point of making the feature useless in my
> eyes. That's really something frequently done.
>

Why not CREATE OR REPLACE TRIGGER? Wouldn't the function itself be an
internal matter rather than something for users to worry about? If the
user needs to adjust it, they'd need to discover the name of the function
the trigger referred to, which may not be trivial.

>
> > And then there are event triggers, which could have the same
> functionality.
>
> I think the need is much less there. You'll hardly create as many even
> triggers as you create triggers on relations. Doesn't seem worth the
> effort.
>

Agreed, but I thought I'd mention it regardless.

--
Thom

On 2014-09-16 13:42:22 +0100, Thom Brown wrote:
> > > The function can't be the target of CREATE OR REPLACE FUNCTION.
> >
> > That *really* sucks. To the point of making the feature useless in my
> > eyes. That's really something frequently done.
> >
>
> Why not CREATE OR REPLACE TRIGGER? Wouldn't the function itself be an
> internal matter rather than something for users to worry about? If the
> user needs to adjust it, they'd need to discover the name of the function
> the trigger referred to, which may not be trivial.

Because CREATE OR REPLACE trigger has to take a heavy relation level
lock? Because we don't have it? Because it'll allow to change things
that you really don't want to change?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 16 September 2014 13:45, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:

> On 2014-09-16 13:42:22 +0100, Thom Brown wrote:
> > > > The function can't be the target of CREATE OR REPLACE FUNCTION.
> > >
> > > That *really* sucks. To the point of making the feature useless in my
> > > eyes. That's really something frequently done.
> > >
> >
> > Why not CREATE OR REPLACE TRIGGER? Wouldn't the function itself be an
> > internal matter rather than something for users to worry about? If the
> > user needs to adjust it, they'd need to discover the name of the function
> > the trigger referred to, which may not be trivial.
>
> Because CREATE OR REPLACE trigger has to take a heavy relation level
> lock? Because we don't have it? Because it'll allow to change things
> that you really don't want to change?

Would CREATE OR REPLACE trigger need a heavy relational level lock if just
the anonymous function body were changing?

My concern is mainly about us on one hand saying "Look, we've removed the
need for trigger statements when creating triggers", then on the other
saying "But if you want to change anything, treat it as if we hadn't done
that, and you'll need to go find the function that we made in the
background."

I guess if we were to do that, we would just need to make it clear that
this is all syntactic sugar, and things like \d+ <table> output would show
the trigger calling the "anonymous" function rather than showing the body
of the function as part of the trigger.

--
Thom

On 2014-09-16 13:54:49 +0100, Thom Brown wrote:
> On 16 September 2014 13:45, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > On 2014-09-16 13:42:22 +0100, Thom Brown wrote:
> > > > > The function can't be the target of CREATE OR REPLACE FUNCTION.
> > > >
> > > > That *really* sucks. To the point of making the feature useless in my
> > > > eyes. That's really something frequently done.
> > > >
> > >
> > > Why not CREATE OR REPLACE TRIGGER? Wouldn't the function itself be an
> > > internal matter rather than something for users to worry about? If the
> > > user needs to adjust it, they'd need to discover the name of the function
> > > the trigger referred to, which may not be trivial.
> >
> > Because CREATE OR REPLACE trigger has to take a heavy relation level
> > lock? Because we don't have it? Because it'll allow to change things
> > that you really don't want to change?
>
>
> Would CREATE OR REPLACE trigger need a heavy relational level lock if just
> the anonymous function body were changing?

I think it's unlikely to change. At the very, very least it'd require a
ShareUpdateExclusive lock on the relation.

> My concern is mainly about us on one hand saying "Look, we've removed the
> need for trigger statements when creating triggers", then on the other
> saying "But if you want to change anything, treat it as if we hadn't done
> that, and you'll need to go find the function that we made in the
> background."

So what? The reason for changing stuff is that it requires superfluous
and annoying typing, right? That's much less the case when you just want
to replace the function's contents after the fact.

> I guess if we were to do that, we would just need to make it clear that
> this is all syntactic sugar, and things like \d+ <table> output would show
> the trigger calling the "anonymous" function rather than showing the body
> of the function as part of the trigger.

I think that should be the case anyway.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services