joey_the_snake

joey_the_snake

Why do you think this? ON CONFLICT DO NOTHING is a database command that avoids raising the error. I have used it in production. PostgreSQL: Documentation: 18: INSERT

The optional ON CONFLICT clause specifies an alternative action to raising a unique violation or exclusion constraint violation error

0
Post #11
trisolaran

trisolaran

You’re right, thanks for the clarification. Sure, no error will be raised, but the DB won’t insert anything in the DB yet insert will return {:ok, post} as if something was inserted.

0
Post #12
joey_the_snake

joey_the_snake

Whether or not that’s fine is up to the developer though, no?

0
Post #13
trisolaran

trisolaran

I personally wouldn’t want that in my code and I would find it confusing if I found it in other codebases. In any case, in this specific example, it’s an issue because @thiagomajesk wans to distinguish between {:ok, post} and {:error, changeset} and be able to take different actions. My point was that he can simplify things and avoid this entire issue by moving his error management code outside of the transaction

0
Post #14
thiagomajesk

thiagomajesk OP

That’s right @trisolaran! Just noticed this behavior testing today… Here’s the log if someone else is curious about it (from the repro I linked in the issue):

10:05:40.814 [debug] QUERY OK db=2.3ms queue=0.3ms
INSERT INTO "tags" VALUES (DEFAULT) RETURNING "id" []

10:05:40.819 [debug] QUERY OK db=2.4ms queue=0.2ms
INSERT INTO "posts" ("title") VALUES ($1) RETURNING "id" ["123"]

10:05:40.819 [debug] QUERY OK db=0.1ms
begin []

10:05:40.819 [debug] QUERY OK db=0.1ms
INSERT INTO "posts" ("title") VALUES ($1) ON CONFLICT DO NOTHING RETURNING "id" ["123"]
POST: %Post{__meta__: #Ecto.Schema.Metadata<:loaded, "posts">, id: nil, title: "123"}

10:05:40.822 [debug] QUERY OK db=0.1ms
commit []

I can see that the error was suppressed correctly but the “post” not persisted (id == nil).
I see how this is unintuitive that we are returning :ok for a failed operation and it wouldn’t be my first choice either. Since this is a corner case, perhaps we could solve it by pattern-matching and just forget about it:

Repo.transaction(fn repo ->
  case repo.insert(changeset) do
    {:ok, %{id: nil}} ->
       # failed first, we don't care anymore
       Repo.rollback(:post)
    {:ok, post} ->    
      tag
      |> Ecto.Changeset.change(%{name: "success"})
      |> repo.update!()
    {:error, changeset} -> 
      tag
      |> Ecto.Changeset.change(%{name: "failure"})
      |> repo.update!()
    end
end)

The real problem with this approach is that we don’t have any information whatsoever about why this has failed and in this specific scenario with no information about the constraint to report to users.

How would you do that if both operations are dependent on each other? I mean, they somehow have to be inside the same transaction to guarantee that we save at least one of them, right!?

BTW I see someone else suggesting the usage of savepoints, but I haven’t used those in Ecto before. I’m not sure how would the fit in this scenario.

0
Post #15
trisolaran

trisolaran

I wonder why you absolutely need to work with these “error” and “failure” tags in the DB and can’t instead log the error or show it to the user?

However, putting this aside, how about this:

  • write a “failure” tag before starting the transaction

Start the transaction:

  • insert the post, if insertion fails, roll back transaction
  • update the “failure” tag to be a “success” tag
  • commit

This way, if anything goes wrong with insertion, you’ll have a failure tag.
Wouldn’t this work?

0
Post #16
thiagomajesk

thiagomajesk OP

Yes, this is absolutely required!

This flow would work, but I’d have to clean all “tags” not referencing “posts” in the DB at the end, which is an operation that could fail on its own and leave us with loose “tags”. Remember that the “tag” in this case, can only exist on two occasions: if a “post” is persisted and if a “post” has errors. It’s almost like a logging mechanism, if I create the log before and there’s nothing worth logging I have to remove it somehow.

BTW, It seems someone else already had this exact same problem: How to not rollback transaction on failed insert because of unique constraint - #8 by benwilson512. I conducted some tests using one of the post’s suggestions and the savepoint feature seems to work without much effort. The only difference is that no additional match on the case was required:

Repo.transaction(fn repo ->   
  case repo.insert(changeset, mode: :savepoint) do
    {:ok, post} ->
      IO.inspect(post, label: "POST")

      tag
      |> Ecto.Changeset.change(%{name: "success"})
      |> repo.update!()

      {:error, changeset} ->
        IO.inspect(changeset, label: "CHANGESET")

        tag
        |> Ecto.Changeset.change(%{name: "failed"})
        |> repo.update!()
    end
end)

This returns the tuple with the changeset containing the constraint errors and commits the transaction as expected:

12:31:46.616 [debug] QUERY OK db=1.6ms queue=0.2ms
INSERT INTO "tags" VALUES (DEFAULT) RETURNING "id" []

12:31:46.620 [debug] QUERY OK db=1.5ms queue=0.1ms
INSERT INTO "posts" ("title") VALUES ($1) RETURNING "id" ["123"]

12:31:46.620 [debug] QUERY OK db=0.1ms
begin []

12:31:46.622 [debug] QUERY ERROR db=1.5ms
INSERT INTO "posts" ("title") VALUES ($1) RETURNING "id" ["123"]
CHANGESET: #Ecto.Changeset<
  action: :insert,
  changes: %{title: "123"},
  errors: [
    title: {"has already been taken",
     [constraint: :unique, constraint_name: "posts_title_index"]}
  ],
  data: #Post<>,
  valid?: false
>

12:31:46.624 [debug] QUERY OK db=0.2ms
UPDATE "tags" SET "name" = $1 WHERE "id" = $2 ["failed", 1]

12:31:46.626 [debug] QUERY OK db=1.8ms
commit []
1
Post #17
joey_the_snake

joey_the_snake

What happens if an exception is thrown when using mode: :savepoint? For instance could this happen:
1. insert succeeds
2. update fails
3. transaction goes to savepoint directly before the update
4. transaction completes successfully

edit: i didn’t notice the savepoint is per statement and not on the transaction. pls ignore.

0
Post #18
Alvinkariuki

Alvinkariuki

Intuitive I like it

1
Post #19
— All posts loaded —