Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

postgres: boss_db:delete() in transaction always rollbacks transaction if no 'counters' table exists in the db #135

Open
brigadier opened this issue Nov 24, 2013 · 3 comments · May be fixed by #206
Open

postgres: boss_db:delete() in transaction always rollbacks transaction if no 'counters' table exists in the db #135

brigadier opened this issue Nov 24, 2013 · 3 comments · May be fixed by #206
Labels
postgresql

Comments

@brigadier
Copy link
Contributor

Postgresql
The following code never succeeds unless there's a 'counters' table in the database:

F = fun() ->
   %% some other things that will allways be rolled back
   boss_db:delete("model-1"); %%never gets deleted, gets rolled back, but returns ok
   boss_db:delete("model-2"); %%throws an exception about rolled back transaction
end,
boss_db:transaction(F)

the same code outside of tranaction works

boss_db:delete("model-1"); %%succeeds

The reason is the function delete:

delete(Conn, Id) when is_list(Id) ->
    {_, TableName, IdColumn, TableId} = boss_sql_lib:infer_type_from_id(Id),
    Res = pgsql:equery(Conn, ["DELETE FROM ", TableName, " WHERE ", IdColumn, " = $1"], [TableId]),
    case Res of
        {ok, _Count} -> 
            pgsql:equery(Conn, "DELETE FROM counters WHERE name = $1", [Id]),
            ok;
        {error, Reason} -> {error, Reason}
    end.

This functions executes the second SQL query which fails and rollbacks the whole transaction.
A result of the second pgsql:equery is ignored so it returns ok every time even then the query fails.

I'm not sure if this should be considered a bug or not.
@davidw
Copy link
Contributor

davidw commented Dec 1, 2014

I just ran into this myself. To me it feels a bit like a bug. The documentation does not describe the counters table as mandatory.

@davidw davidw self-assigned this Dec 1, 2014
davidw added a commit to davidw/boss_db that referenced this issue Dec 1, 2014
@davidw
Separate delete from delete_counter
d35c7e4 
Using delete for two different things - deleting records and deleting
keys from the counter table - was causing problems like:

ErlyORM#135

This commit introduces a delete_counter that only deletes keys from
the counter.  The delete function only deletes records.
@davidw davidw linked a pull request Dec 1, 2014 that will close this issue
Open
@timClicks
Copy link

Hrm I always thought that a counters table was compulsory.. and seem to recall reading docs about it. I do remember being bitten by something similiar when I was trying to get up and running. Perhaps it was just the auto-initialisation scripts created them.

@solatis
Copy link
Contributor

solatis commented Sep 16, 2015

This issue should be fixed. To be honest, I feel like that the counters functionality is better off as a separate library/plugin for boss_db, and perhaps make it compulsory if you want to use that plugin.

A workaround for the time being could be to do a table_exists of the counters table when doing a delete, but in no way should this be exposed to the developer that doesn't use counters. I would be willing to address this issue myself, once we reach agreement on what the solution should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
postgresql
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Separate delete from delete_counter davidw/boss_db
4 participants
@davidw @timClicks @solatis @brigadier