Thread: isn't "insert into where not exists" atomic?
Hello,
I just received an error message:
PGError: ERROR: duplicate key value violates unique constraint "chu_user_id_chat_room_id"
DETAIL: Key (user_id, chat_room_id)=(8, 2) already exists.
CONTEXT: SQL statement "insert into chat_room_users (user_id, chat_room_id, active_at) (select NEW.user_id,
NEW.chat_room_id,now() where not exists (select 1 from chat_room_users where user_id = NEW.user_id and chat_room_id =
NEW.chat_room_id))"
PL/pgSQL function "trf_chat_room_users_insert" line 3 at SQL statement
: INSERT INTO "chat_room_users" ("user_id", "chat_room_id", "active_at") VALUES (8, 2, NULL)
The important line is:
insert into chat_room_users (user_id, chat_room_id, active_at) (select NEW.user_id, NEW.chat_room_id, now() where not
exists(select 1 from chat_room_users where user_id = NEW.user_id and chat_room_id = NEW.chat_room_id))
I always thought this is atomic and can not fail. Was I wrong?
If it isn't then I have to rewrite my triggers. Do I have to use "lock
table" instead of the above to avoid errors in parallel inserts?
The trigger looks like:
create or replace function trf_chat_room_users_insert() returns trigger
as $$
begin
if NEW.active_at is null then
insert into chat_room_users (user_id, chat_room_id,
active_at) (select NEW.user_id, NEW.chat_room_id, now() where not exists
(select 1 from chat_room_users where user_id = NEW.user_id and
chat_room_id = NEW.chat_room_id));
if not found then
update chat_room_users set active_at = now()
where user_id = NEW.user_id and chat_room_id = NEW.chat_room_id;
end if;
return null;
end if;
return NEW;
end;
$$ language plpgsql;
And it meant to be "insert or update".
Mage
On 3 Feb 2011, at 2:17, Mage wrote: > The trigger looks like: > > create or replace function trf_chat_room_users_insert() returns trigger as $$ > begin > if NEW.active_at is null then > insert into chat_room_users (user_id, chat_room_id, active_at) (select NEW.user_id, NEW.chat_room_id, now()where not exists (select 1 from chat_room_users where user_id = NEW.user_id and chat_room_id = NEW.chat_room_id)); > if not found then > update chat_room_users set active_at = now() where user_id = NEW.user_id and chat_room_id = NEW.chat_room_id; > end if; > return null; > end if; > return NEW; > end; > $$ language plpgsql; Your trigger is the wrong way around. Insert doesn't set found, but update does. Alban Hertroys -- If you can't see the forest for the trees, cut the trees and you'll see there is no forest. !DSPAM:737,4d4a559711736475013765!
On 02/03/2011 08:13 AM, Alban Hertroys wrote:
> On 3 Feb 2011, at 2:17, Mage wrote:
>
>> The trigger looks like:
>>
>> create or replace function trf_chat_room_users_insert() returns trigger as $$
>> begin
>> if NEW.active_at is null then
>> insert into chat_room_users (user_id, chat_room_id, active_at) (select NEW.user_id,
NEW.chat_room_id,now() where not exists (select 1 from chat_room_users where user_id = NEW.user_id and chat_room_id =
NEW.chat_room_id));
>> if not found then
>> update chat_room_users set active_at = now() where user_id = NEW.user_id and chat_room_id =
NEW.chat_room_id;
>> end if;
>> return null;
>> end if;
>> return NEW;
>> end;
>> $$ language plpgsql;
>
> Your trigger is the wrong way around. Insert doesn't set found, but update does.
>
> Alban Hertroys
I think you missed the point that the insert contains a select which
sets found.
My trigger works fine and it was called thousands times. It just dropped
an exception two times.
The main question is that isn't "insert into ... select ... where not
exists" atomic?
Anyway, it you'd try it:
create table chat_room_users (
user_id int not null,
chat_room_id int not null,
active_at timestamp with time zone not null
);
create unique index chu_user_id_chat_room_id on chat_room_users
(user_id, chat_room_id);
create or replace function trf_chat_room_users_insert() returns trigger
as $$
begin
if NEW.active_at is null then
insert into chat_room_users (user_id, chat_room_id, active_at)
(select NEW.user_id, NEW.chat_room_id, now() where not exists (select 1
from chat_room_users where user_id = NEW.user_id and chat_room_id =
NEW.chat_room_id));
if not found then
update chat_room_users set active_at = now() where user_id
= NEW.user_id and chat_room_id = NEW.chat_room_id;
end if;
return null;
end if;
return NEW;
end;
$$ language plpgsql;
create trigger tr_chat_room_users_insert before insert on
chat_room_users for each row execute procedure trf_chat_room_users_insert();
insert into chat_room_users (user_id, chat_room_id) values (1, 1);
insert into chat_room_users (user_id, chat_room_id) values (2, 1);
insert into chat_room_users (user_id, chat_room_id) values (1, 1);
insert into chat_room_users (user_id, chat_room_id) values (2, 1);
Mage
Mage <mage@mage.hu> writes: > The main question is that isn't "insert into ... select ... where not > exists" atomic? No, it isn't: it *will* fail in the presence of other transactions doing the same thing, because the EXISTS test will only see rows that committed before the command started. You might care to read the manual's chapter about concurrency: http://www.postgresql.org/docs/9.0/static/mvcc.html regards, tom lane
Your trigger is wrong. You try to insert the same row twice.
On 02/03/2011 10:35 PM, pasman pasmański wrote:
> Your trigger is wrong. You try to insert the same row twice.
>
I assume you didn't try it. If active_at field is null then the trigger
does another insert instead of the original one. This avoids looping or
inserting twice.
The only mistake is that I tought the insert with select will be atomic
and I was wrong. So the trigger has concurrency problem in
multi-threaded environment. It runs flawlessly in single thread and it
does only a single insert.
Mage
On 02/03/2011 08:23 PM, Tom Lane wrote: > Mage<mage@mage.hu> writes: >> The main question is that isn't "insert into ... select ... where not >> exists" atomic? > No, it isn't: it *will* fail in the presence of other transactions doing > the same thing, because the EXISTS test will only see rows that > committed before the command started. You might care to read the > manual's chapter about concurrency: > http://www.postgresql.org/docs/9.0/static/mvcc.html Thank you, Tom. I will read that. However I googled a bit before written this trigger and I would like to ask you: what is the best practice for doing "insert or update"-like thing, especially in this case, in trigger? I would use lock table from now. Is it the recommended way? (I just don't like the "insert -> on exception -> update" method). Mage
Mage <mage@mage.hu> writes: > On 02/03/2011 08:23 PM, Tom Lane wrote: >> No, it isn't: it *will* fail in the presence of other transactions doing >> the same thing, because the EXISTS test will only see rows that >> committed before the command started. You might care to read the >> manual's chapter about concurrency: >> http://www.postgresql.org/docs/9.0/static/mvcc.html > Thank you, Tom. I will read that. > However I googled a bit before written this trigger and I would like to > ask you: what is the best practice for doing "insert or update"-like > thing, especially in this case, in trigger? I would use lock table from > now. Is it the recommended way? > (I just don't like the "insert -> on exception -> update" method). AFAIR the basic alternatives are insert -> exception -> update or taking a lock at the table level. The latter is simpler and cleaner but distinctly worse for concurrent-insert performance, especially if you can't keep the transactions very short. Pick your poison ... regards, tom lane
Mage, add "raise notice" at the begin of your buggy trigger.
On 02/04/2011 06:57 AM, pasman pasmański wrote:
> Mage, add "raise notice" at the begin of your buggy trigger.
>
There is a little bit of difference between "Your trigger is wrong. You
try to insert the same row twice" and "The trigger will be fired twice."
You stated the first but the second is the truth.
Nevermind, my solution will be using table level lock for this case.
Mage