Skip to content

Conversation

@jrochel
Copy link

@jrochel jrochel commented Jul 8, 2016

No description provided.

@jrochel jrochel force-pushed the ocsipersist-pgsql branch from 0bf3664 to 8e5e467 Compare July 8, 2016 14:50
@jrochel jrochel force-pushed the ocsipersist-pgsql branch 2 times, most recently from e8b0e2b to c962748 Compare July 11, 2016 09:57
@jrochel jrochel force-pushed the ocsipersist-pgsql branch from c962748 to 23b1bf7 Compare July 11, 2016 09:59
let open_store store = use_pool @@ fun db ->
create_table db store >> Lwt.return store

let make_persistent_lazy_lwt ~store ~name ~default = use_pool @@ fun db ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is racy. Another thread could insert into the table after the select and before we perform the insert ourself.
I think, the best we can do is to repeat the two following commands until one of them succeed (that is, until it returns a non-empty list).

SELECT value FROM %s WHERE key = $1;
INSERT INTO %s VALUES ($1, $2) ON CONFLICT (key) DO NOTHING RETURNING 0;

For compatibility with PostgreSQL < 9.5, we can probably replace the second command with the following one and catch the duplicate-key error:

INSERT INTO %s VALUES ($1, $2);

@jrochel jrochel force-pushed the ocsipersist-pgsql branch from 21e3795 to 8064c11 Compare July 11, 2016 12:46
@jrochel jrochel force-pushed the ocsipersist-pgsql branch from 92f2e4a to b0f08ea Compare July 11, 2016 13:14
@jrochel jrochel force-pushed the ocsipersist-pgsql branch from 7d39b76 to 1823669 Compare July 11, 2016 13:50
@vouillon vouillon merged commit fbdeb1c into ocsigen:master Jul 12, 2016
and depends on {{{ sqlite3.cma }}}).
{{{ ocsipersist-dbm.cma }}} (uses the DBM database);
{{{ ocsipersist-sqlite.cma }}} (uses the SQLite database
and depends on {{{ sqlite3.cma }}});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't be better to point to the opam package name ... ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants