5 ms·

Except that that's not concurrency safe in a transaction, because you might not yet see the row inside the NOT EXISTS() subquery if it hasn't yet committed.

jessaustin 8y ago
The update isn't inserting anything. Either the row exists already, or it doesn't. Maybe I'm missing something?
- anarazel 8y ago
  > The update isn't inserting anything. Either the row exists already, or it doesn't. Maybe I'm missing something? Rows can exists without you being able to see them. Consider: S1: BEGIN; S2: BEGIN; S1: INSERT name = 'Alice' S2: UPDATE WHERE name = 'Alice' -> no row matches S2: INSERT name = 'Alice' WHERE NOT EXISTS (name = 'Alice') S2: <blocks, due to potential uniqueness violation> S1: COMMIT; S2: <resumes, raises uniqueness violation> Edit: formatting
  - eclark 8y ago
    I haven't tested but I don't think that will happen on the default settings for postgres. I know for a fact that it won't on higher isolation levels. https://www.postgresql.org/docs/9.1/static/transaction-iso.html https://www.postgresql.org/docs/9.1/static/transaction-iso.h... > If the first updater commits, the second updater will ignore the row if the first updater deleted it, otherwise it will attempt to apply its operation to the updated version of the row. My understanding is that the update from S2 would be rerun applied. Specifically postgres calls out: > The search condition of the command (the WHERE clause) is re-evaluated to see if the updated version of the row still matches the search condition.
    - anarazel 8y ago
      > I haven't tested but I don't think that will happen on the default settings for postgres. I know for a fact that it won't on higher isolation levels. Unfortunately you're wrong. I've now tested it, but I was also pretty confident before - I'm a postgres developer, and I worked on/comitted the PG upsert implementation ;) postgres[10287][1]=# CREATE TABLE data(key text unique); CREATE TABLE postgres[10287][1]=# BEGIN ISOLATION LEVEL SERIALIZABLE ; BEGIN postgres[10284][1]=# BEGIN ISOLATION LEVEL SERIALIZABLE ; BEGIN postgres[10287][1]*=# INSERT INTO data VALUES('alice'); INSERT 0 1 postgres[10284][1]*=# INSERT INTO data SELECT 'alice' WHERE NOT EXISTS(SELECT * FROM data WHERE key = 'alice'); postgres[10287][1]*=# COMMIT; COMMIT postgres[10284][1]*=# ERROR: 23505: duplicate key value violates unique constraint "data_key_key" DETAIL: Key (key)=(alice) already exists. SCHEMA NAME: public TABLE NAME: data CONSTRAINT NAME: data_key_key LOCATION: _bt_check_unique, nbtinsert.c:535 (the number in brackets in the prompt is the backend pid, allowing to differentiate the two sessions). > > If the first updater commits, the second updater will ignore the row if the first updater deleted it, otherwise it will attempt to apply its operation to the updated version of the row. > My understanding is that the update from S2 would be rerun applied. > Specifically postgres calls out: > > The search condition of the command (the WHERE clause) is re-evaluated to see if the updated version of the row still matches the search condition. Those comments are about row-level locks - they're not the problem here. What you get is a constraint violation due to the unique constraint.
      - chacham15 8y ago
        I know very little about Postgres, but doesnt this appear to violate the isolation property of ACID?
        anarazel 8y ago
        No. ACID doesn't guarantee that you can't have constraint violations triggered by a concurrent session. That'd make any sort of efficient constraint pretty much impossible. It guarantees that that error is caught despite the concurrency however.
        8y ago
    - tscs37 8y ago
      Usually for the serializable isolation level you need to manually rerun the transaction. There are some DBs that do that for you like CockroachDB.
- aseipp 8y ago
  I believe the issue is a concurrent transaction (i.e. a separate one from the one you're currently in) may perform the INSERT and COMMIT that insert after your transaction's SELECT subquery returns nothing (thus allowing INSERT to proceed, by the WHERE clause), but before the actual INSERT itself is committed. This means there is a window between the sub-SELECT and the INSERT itself where a secondary transaction can insert something, causing both your original update to do nothing, and the insert to fail. Thus you've lost the write entirely.
  - jessaustin 8y ago
    In that case the other effort (I'm reluctant to call it a transaction) wins. Maybe it was later and should win, maybe it was earlier and shouldn't. It takes a lot more care than seen here to preserve a meaning of "earlier" and "later" that isn't vulnerable to this issue. The way the schema is designed, there can be only one "Alice". I'm trying and failing to imagine a way in which both this effort and the other effort can fail.
    - aseipp 8y ago
      I think the particular point made by Andres (and elsewhere in this thread) is really not about what the schema for a phone book is like, and more that "Accurately replicating what UPSERT does without it is exceedingly hard and you will probably get it wrong or suboptimal in a number of scenarios you can't possibly imagine until they happen". Isolation levels will for example have impact on how transactions proceed and what results you see (REPEATABLE READ vs READ COMMITTED); the error handling (did your transaction actually fail from a concurrent INSERT succeeding, or did the update fail, or did something else happen entirely? UPSERT is often not a standalone operation in a transaction) must be carefully decided for each use-case or else you're likely to redo needless transactions or throw away good ones, and you have to work around incidental problems like latency using stored procedures to keep everything server-side, etc etc. It's just not the kind of thing you want to replicate a number of places at each use site in a number of tools. It's something only the database knows how to manage properly by having "global knowledge".
thr0awaz 8y ago
I'm not 100% familiar with the system, but depending on the transaction levels available you could do wrap it in like... a serializable transaction, then select the rows (just to lock them), then do the update, then do the insert, then commit the transaction Maybe... I'm used to systems with upsert/merge.
cobythedog 8y ago
But we are talking about sqlite, right? By default the database is locked once a write occurs, so there could not be concurrent transactions writing to the database.

jessaustin 8y ago

The update isn't inserting anything. Either the row exists already, or it doesn't. Maybe I'm missing something?

anarazel 8y ago
> The update isn't inserting anything. Either the row exists already, or it doesn't. Maybe I'm missing something? Rows can exists without you being able to see them. Consider: S1: BEGIN; S2: BEGIN; S1: INSERT name = 'Alice' S2: UPDATE WHERE name = 'Alice' -> no row matches S2: INSERT name = 'Alice' WHERE NOT EXISTS (name = 'Alice') S2: <blocks, due to potential uniqueness violation> S1: COMMIT; S2: <resumes, raises uniqueness violation> Edit: formatting
- eclark 8y ago
  I haven't tested but I don't think that will happen on the default settings for postgres. I know for a fact that it won't on higher isolation levels. https://www.postgresql.org/docs/9.1/static/transaction-iso.html https://www.postgresql.org/docs/9.1/static/transaction-iso.h... > If the first updater commits, the second updater will ignore the row if the first updater deleted it, otherwise it will attempt to apply its operation to the updated version of the row. My understanding is that the update from S2 would be rerun applied. Specifically postgres calls out: > The search condition of the command (the WHERE clause) is re-evaluated to see if the updated version of the row still matches the search condition.
  - anarazel 8y ago
    > I haven't tested but I don't think that will happen on the default settings for postgres. I know for a fact that it won't on higher isolation levels. Unfortunately you're wrong. I've now tested it, but I was also pretty confident before - I'm a postgres developer, and I worked on/comitted the PG upsert implementation ;) postgres[10287][1]=# CREATE TABLE data(key text unique); CREATE TABLE postgres[10287][1]=# BEGIN ISOLATION LEVEL SERIALIZABLE ; BEGIN postgres[10284][1]=# BEGIN ISOLATION LEVEL SERIALIZABLE ; BEGIN postgres[10287][1]*=# INSERT INTO data VALUES('alice'); INSERT 0 1 postgres[10284][1]*=# INSERT INTO data SELECT 'alice' WHERE NOT EXISTS(SELECT * FROM data WHERE key = 'alice'); postgres[10287][1]*=# COMMIT; COMMIT postgres[10284][1]*=# ERROR: 23505: duplicate key value violates unique constraint "data_key_key" DETAIL: Key (key)=(alice) already exists. SCHEMA NAME: public TABLE NAME: data CONSTRAINT NAME: data_key_key LOCATION: _bt_check_unique, nbtinsert.c:535 (the number in brackets in the prompt is the backend pid, allowing to differentiate the two sessions). > > If the first updater commits, the second updater will ignore the row if the first updater deleted it, otherwise it will attempt to apply its operation to the updated version of the row. > My understanding is that the update from S2 would be rerun applied. > Specifically postgres calls out: > > The search condition of the command (the WHERE clause) is re-evaluated to see if the updated version of the row still matches the search condition. Those comments are about row-level locks - they're not the problem here. What you get is a constraint violation due to the unique constraint.
    - chacham15 8y ago
      I know very little about Postgres, but doesnt this appear to violate the isolation property of ACID?
      - anarazel 8y ago
        No. ACID doesn't guarantee that you can't have constraint violations triggered by a concurrent session. That'd make any sort of efficient constraint pretty much impossible. It guarantees that that error is caught despite the concurrency however.
        8y ago
  - tscs37 8y ago
    Usually for the serializable isolation level you need to manually rerun the transaction. There are some DBs that do that for you like CockroachDB.
aseipp 8y ago
I believe the issue is a concurrent transaction (i.e. a separate one from the one you're currently in) may perform the INSERT and COMMIT that insert after your transaction's SELECT subquery returns nothing (thus allowing INSERT to proceed, by the WHERE clause), but before the actual INSERT itself is committed. This means there is a window between the sub-SELECT and the INSERT itself where a secondary transaction can insert something, causing both your original update to do nothing, and the insert to fail. Thus you've lost the write entirely.
- jessaustin 8y ago
  In that case the other effort (I'm reluctant to call it a transaction) wins. Maybe it was later and should win, maybe it was earlier and shouldn't. It takes a lot more care than seen here to preserve a meaning of "earlier" and "later" that isn't vulnerable to this issue. The way the schema is designed, there can be only one "Alice". I'm trying and failing to imagine a way in which both this effort and the other effort can fail.
  - aseipp 8y ago
    I think the particular point made by Andres (and elsewhere in this thread) is really not about what the schema for a phone book is like, and more that "Accurately replicating what UPSERT does without it is exceedingly hard and you will probably get it wrong or suboptimal in a number of scenarios you can't possibly imagine until they happen". Isolation levels will for example have impact on how transactions proceed and what results you see (REPEATABLE READ vs READ COMMITTED); the error handling (did your transaction actually fail from a concurrent INSERT succeeding, or did the update fail, or did something else happen entirely? UPSERT is often not a standalone operation in a transaction) must be carefully decided for each use-case or else you're likely to redo needless transactions or throw away good ones, and you have to work around incidental problems like latency using stored procedures to keep everything server-side, etc etc. It's just not the kind of thing you want to replicate a number of places at each use site in a number of tools. It's something only the database knows how to manage properly by having "global knowledge".

thr0awaz 8y ago

I'm not 100% familiar with the system, but depending on the transaction levels available you could do wrap it in like... a serializable transaction, then select the rows (just to lock them), then do the update, then do the insert, then commit the transaction Maybe... I'm used to systems with upsert/merge.

cobythedog 8y ago

But we are talking about sqlite, right? By default the database is locked once a write occurs, so there could not be concurrent transactions writing to the database.