d2km / raw_sqlite3 Goto Github PK

According to the spec (https://github.com/d2km/raw_sqlite3/blob/master/src/raw_sqlite3.erl#L206), prepare/2 should return an {ok, Stmt}, but it actually returns {ok, Stmt, Leftover}.

The relevant nif function in sqlite3_nif.c returns the following:

 return enif_make_tuple3(env, make_atom(env, "ok"), ret_stmt, ret_leftover);

This is because the sqlite3 API only prepares 1 statement per query, so if you give a query with multiple statements to sqlite3_prepare_v2 it will prepare the first statement and point the leftover pointer to the remaining statements as a string. For example:

4> Query = "SELECT foo FROM table; SELECT bar FROM table",
4> raw_sqlite3:prepare(Conn, Query).
{ok,#Ref<0.2897351509.2542403607.152620>,
    <<" SELECT bar FROM table">>}

Happy to make a PR to fix this but I see a couple of options:

1. Update the spec for prepare/2 so that it returns {ok, Stmt, Leftover}, or;
2. Return {ok, Stmt} in prepare/2 if the leftover returned by the nif function impl_sqlite3_prepare_v2 is an empty string and return {ok, Stmt, Leftover} otherwise.

Thoughts?

Due to some performance regression in OTP21+ when using file:read_line and utilizing read ahead buffer (see https://bugs.erlang.org/browse/ERL-1347) I have tested the library in OTP20. There is precisely one line of code preventing it from compiling:

The above clause head from a try expression uses a syntax for retrieving the stacktrace that was introduced in OTP21 (see the relevant EEP here: https://www.erlang.org/erlang-enhancement-proposals/eep-0047.html) but is not compatible with previous versions of OTP.

Since this is the only line making the library incompatible with OTP20, it might be possible to make it work with OTP20+ with little effort.

The alternative method of retrieving the stacktrace from within the same scope as the relevant try expression in OTP is erlang:get_stacktrace/0. However, this is already incompatible with OTP23 (returns an empty list) and is scheduled for removal in OTP24 (see https://www.erlang.org/news/140).

Therefore, I suggest using the OTP_RELEASE macro (see https://www.erlang.org/erlang-enhancement-proposals/eep-0044.html) to retrieve the stacktrace in the relevant part of the code using either of the above methods depending on the OTP version.

Note also that dirty schedulers for NIFs were introduced in OTP20 (https://www.erlang.org/news/114), so this is the earliest possible version that we could easily support.

What do you think @d2km ?

Currently reset/2 just gives the error code returned by the underlying nif function to raw_sqlite3:expand_error, which returns {ok, SQLITE_OK} on success.

I think it makes sense for it to return simply ok instead. This also makes it consistent with step, bind, exec, and insert_many.

Thoughts? Happy to make a PR if you agree.