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

Add individual binds #298

Merged
merged 10 commits into from
Oct 9, 2024
Merged

Add individual binds #298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e1f7d87
add individual binds
ruslandoga Oct 9, 2024
895de04
make credo happy
ruslandoga Oct 9, 2024
1711dad
re-implement bind/3
ruslandoga Oct 9, 2024
25ea4f2
deprecate bind/3 in favor of bind/2
ruslandoga Oct 9, 2024
a0d98e9
use raise_badarg() in errstr and errmsg as well
ruslandoga Oct 9, 2024
9870daa
make credo ignore bind_all
ruslandoga Oct 9, 2024
e5f6094
wrap bind/2 in try
ruslandoga Oct 9, 2024
5640d4c
add bind/2 doc
ruslandoga Oct 9, 2024
c384bd1
add reset/1
ruslandoga Oct 9, 2024
fa2cff4
remove argc asserts
ruslandoga Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
292 changes: 131 additions & 161 deletions c_src/sqlite3_nif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,194 +448,124 @@ exqlite_prepare(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
}

static ERL_NIF_TERM
bind(ErlNifEnv* env, const ERL_NIF_TERM arg, sqlite3_stmt* statement, int index)
raise_badarg(ErlNifEnv* env, ERL_NIF_TERM term)
{
int rc;
int the_int;
ErlNifSInt64 the_long_int;
double the_double;
char the_atom[MAX_ATOM_LENGTH + 1];
ErlNifBinary the_blob;
int arity;
const ERL_NIF_TERM* tuple;

if (enif_get_int64(env, arg, &the_long_int)) {
rc = sqlite3_bind_int64(statement, index, the_long_int);
if (rc == SQLITE_OK) {
return make_atom(env, "ok");
}

return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument as 64 bit integer"),
arg));
}
ERL_NIF_TERM badarg = enif_make_tuple2(env, make_atom(env, "badarg"), term);
return enif_raise_exception(env, badarg);
}

if (enif_get_int(env, arg, &the_int)) {
rc = sqlite3_bind_int(statement, index, the_int);
if (rc == SQLITE_OK) {
return make_atom(env, "ok");
}
static ERL_NIF_TERM
exqlite_reset(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
statement_t* statement;
if (!enif_get_resource(env, argv[0], statement_type, (void**)&statement))
return raise_badarg(env, argv[0]);

return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument as integer"),
arg));
}
sqlite3_reset(statement->statement);
return make_atom(env, "ok");
}

if (enif_get_double(env, arg, &the_double)) {
rc = sqlite3_bind_double(statement, index, the_double);
if (rc == SQLITE_OK) {
return make_atom(env, "ok");
}
static ERL_NIF_TERM
exqlite_bind_parameter_count(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{

return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument as double"),
arg));
}
statement_t* statement;
if (!enif_get_resource(env, argv[0], statement_type, (void**)&statement))
return raise_badarg(env, argv[0]);

if (enif_get_atom(env, arg, the_atom, sizeof(the_atom), ERL_NIF_LATIN1)) {
if (0 == strcmp("undefined", the_atom) || 0 == strcmp("nil", the_atom)) {
rc = sqlite3_bind_null(statement, index);
if (rc == SQLITE_OK) {
return make_atom(env, "ok");
}
int bind_parameter_count = sqlite3_bind_parameter_count(statement->statement);
return enif_make_int(env, bind_parameter_count);
}

return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument as null"),
arg));
}
static ERL_NIF_TERM
exqlite_bind_text(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
statement_t* statement;
if (!enif_get_resource(env, argv[0], statement_type, (void**)&statement))
return raise_badarg(env, argv[0]);

rc = sqlite3_bind_text(statement, index, the_atom, strlen(the_atom), SQLITE_TRANSIENT);
if (rc == SQLITE_OK) {
return make_atom(env, "ok");
}
unsigned int idx;
if (!enif_get_uint(env, argv[1], &idx))
return raise_badarg(env, argv[1]);

return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument as text"),
arg));
}
ErlNifBinary text;
if (!enif_inspect_binary(env, argv[2], &text))
return raise_badarg(env, argv[2]);

if (enif_inspect_iolist_as_binary(env, arg, &the_blob)) {
rc = sqlite3_bind_text(statement, index, (char*)the_blob.data, the_blob.size, SQLITE_TRANSIENT);
if (rc == SQLITE_OK) {
return make_atom(env, "ok");
}
int rc = sqlite3_bind_text(statement->statement, idx, (char*)text.data, text.size, SQLITE_TRANSIENT);
return enif_make_int(env, rc);
}

return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument as text"),
arg));
}
static ERL_NIF_TERM
exqlite_bind_blob(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
statement_t* statement;
if (!enif_get_resource(env, argv[0], statement_type, (void**)&statement))
return raise_badarg(env, argv[0]);

if (enif_get_tuple(env, arg, &arity, &tuple)) {
if (arity != 2) {
return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument as blob"),
arg));
}
unsigned int idx;
if (!enif_get_uint(env, argv[1], &idx))
return raise_badarg(env, argv[1]);

if (enif_get_atom(env, tuple[0], the_atom, sizeof(the_atom), ERL_NIF_LATIN1)) {
if (0 == strcmp("blob", the_atom)) {
if (enif_inspect_iolist_as_binary(env, tuple[1], &the_blob)) {
rc = sqlite3_bind_blob(statement, index, the_blob.data, the_blob.size, SQLITE_TRANSIENT);
if (rc == SQLITE_OK) {
return make_atom(env, "ok");
}

return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument as blob"),
arg));
}
}
}
}
ErlNifBinary blob;
if (!enif_inspect_binary(env, argv[2], &blob))
return raise_badarg(env, argv[2]);

return enif_raise_exception(
env,
make_bind_error(
env,
make_message(env, "Failed to bind argument"),
arg));
int rc = sqlite3_bind_blob(statement->statement, idx, (char*)blob.data, blob.size, SQLITE_TRANSIENT);
return enif_make_int(env, rc);
}

///
/// @brief Binds arguments to the sql statement
///
static ERL_NIF_TERM
exqlite_bind(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
exqlite_bind_integer(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
assert(env);
statement_t* statement;
if (!enif_get_resource(env, argv[0], statement_type, (void**)&statement))
return raise_badarg(env, argv[0]);

unsigned int parameter_count = 0;
unsigned int argument_list_length = 0;
connection_t* conn = NULL;
statement_t* statement = NULL;
ERL_NIF_TERM list;
ERL_NIF_TERM head;
ERL_NIF_TERM tail;
unsigned int idx;
if (!enif_get_uint(env, argv[1], &idx))
return raise_badarg(env, argv[1]);

if (argc != 3) {
return enif_make_badarg(env);
}

if (!enif_get_resource(env, argv[0], connection_type, (void**)&conn)) {
return make_error_tuple(env, "invalid_connection");
}

if (!enif_get_resource(env, argv[1], statement_type, (void**)&statement)) {
return make_error_tuple(env, "invalid_statement");
}
ErlNifSInt64 i;
if (!enif_get_int64(env, argv[2], &i))
return raise_badarg(env, argv[2]);

if (!enif_get_list_length(env, argv[2], &argument_list_length)) {
return make_error_tuple(env, "bad_argument_list");
}
int rc = sqlite3_bind_int64(statement->statement, idx, i);
return enif_make_int(env, rc);
}

parameter_count = (unsigned int)sqlite3_bind_parameter_count(statement->statement);
if (parameter_count != argument_list_length) {
return make_error_tuple(env, "arguments_wrong_length");
}
static ERL_NIF_TERM
exqlite_bind_float(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
statement_t* statement;
if (!enif_get_resource(env, argv[0], statement_type, (void**)&statement))
return raise_badarg(env, argv[0]);

sqlite3_reset(statement->statement);
ruslandoga marked this conversation as resolved.
Show resolved Hide resolved
unsigned int idx;
if (!enif_get_uint(env, argv[1], &idx))
return raise_badarg(env, argv[1]);

list = argv[2];
for (unsigned int i = 0; i < argument_list_length; i++) {
enif_get_list_cell(env, list, &head, &tail);
ERL_NIF_TERM result = bind(env, head, statement->statement, i + 1);
double f;
if (!enif_get_double(env, argv[2], &f))
return raise_badarg(env, argv[2]);

// We are going to ignore this, we have to pass it.
ERL_NIF_TERM reason;
int rc = sqlite3_bind_double(statement->statement, idx, f);
return enif_make_int(env, rc);
}

// Bind will set an exception if anything happens during that phase.
if (enif_has_pending_exception(env, &reason)) {
return make_error_tuple(env, "failed_to_bind_argument");
}
static ERL_NIF_TERM
exqlite_bind_null(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
statement_t* statement;
if (!enif_get_resource(env, argv[0], statement_type, (void**)&statement))
return raise_badarg(env, argv[0]);

list = tail;
}
unsigned int idx;
if (!enif_get_uint(env, argv[1], &idx))
return raise_badarg(env, argv[1]);

return make_atom(env, "ok");
int rc = sqlite3_bind_null(statement->statement, idx);
return enif_make_int(env, rc);
}

static ERL_NIF_TERM
Expand Down Expand Up @@ -1272,6 +1202,38 @@ exqlite_interrupt(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
return make_atom(env, "ok");
}

static ERL_NIF_TERM
exqlite_errmsg(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
connection_t* conn;
statement_t* statement;
const char* msg;

if (enif_get_resource(env, argv[0], connection_type, (void**)&conn)) {
msg = sqlite3_errmsg(conn->db);
} else if (enif_get_resource(env, argv[0], statement_type, (void**)&statement)) {
msg = sqlite3_errmsg(sqlite3_db_handle(statement->statement));
} else {
return raise_badarg(env, argv[0]);
}

if (!msg)
return make_atom(env, "nil");

return make_binary(env, msg, strlen(msg));
}

static ERL_NIF_TERM
exqlite_errstr(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
{
int rc;
if (!enif_get_int(env, argv[0], &rc))
return raise_badarg(env, argv[0]);

const char* msg = sqlite3_errstr(rc);
return make_binary(env, msg, strlen(msg));
}

//
// Most of our nif functions are going to be IO bounded
//
Expand All @@ -1282,7 +1244,13 @@ static ErlNifFunc nif_funcs[] = {
{"execute", 2, exqlite_execute, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"changes", 1, exqlite_changes, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"prepare", 2, exqlite_prepare, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"bind", 3, exqlite_bind, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"reset", 1, exqlite_reset, ERL_NIF_DIRTY_JOB_CPU_BOUND},
Copy link
Contributor Author

@ruslandoga ruslandoga Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm using a dirty CPU scheduler here because of this mutex: https://github.com/sqlite/sqlite/blob/7f5a10e4ba20826b03ea898945f3ae9138b5568e/src/vdbeapi.c#L136

It can probably be scheduled on the main scheduler in most db_connection-related scenarios though. Or we can use https://www.sqlite.org/c3ref/stmt_status.html to check if the statement needs reset (i.e. if run > 1), and only run it then. sqlite3_stmt_status can use a main scheduler since it's just a struct field read.

Copy link
Member

Choose a reason for hiding this comment

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

Yea this is fine

{"bind_parameter_count", 1, exqlite_bind_parameter_count},
{"bind_text", 3, exqlite_bind_text},
{"bind_blob", 3, exqlite_bind_blob},
{"bind_integer", 3, exqlite_bind_integer},
{"bind_float", 3, exqlite_bind_float},
{"bind_null", 2, exqlite_bind_null},
{"step", 2, exqlite_step, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"multi_step", 3, exqlite_multi_step, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"columns", 2, exqlite_columns, ERL_NIF_DIRTY_JOB_IO_BOUND},
Expand All @@ -1295,6 +1263,8 @@ static ErlNifFunc nif_funcs[] = {
{"set_update_hook", 2, exqlite_set_update_hook, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"set_log_hook", 1, exqlite_set_log_hook, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"interrupt", 1, exqlite_interrupt, ERL_NIF_DIRTY_JOB_IO_BOUND},
{"errmsg", 1, exqlite_errmsg},
{"errstr", 1, exqlite_errstr},
};

ERL_NIF_INIT(Elixir.Exqlite.Sqlite3NIF, nif_funcs, on_load, NULL, NULL, on_unload)
18 changes: 0 additions & 18 deletions lib/exqlite/bind_error.ex
View file
Open in desktop

This file was deleted.

12 changes: 6 additions & 6 deletions lib/exqlite/connection.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,12 +652,12 @@ defmodule Exqlite.Connection do

defp bind_params(%Query{ref: ref, statement: statement} = query, params, state)
when ref != nil do
case Sqlite3.bind(state.db, ref, params) do
:ok ->
{:ok, query}

{:error, reason} ->
{:error, %Error{message: to_string(reason), statement: statement}, state}
try do
Sqlite3.bind(ref, params)
rescue
e -> {:error, %Error{message: Exception.message(e), statement: statement}, state}
Copy link
Contributor Author

@ruslandoga ruslandoga Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We lose useful stacktrace here (which bind_* was called? helps identify which type was expected), but this can probably be handled later.

Copy link
Member

Choose a reason for hiding this comment

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

Can we reraise with a reformed error and retain the stack trace?

Copy link
Contributor Author

@ruslandoga ruslandoga Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe. I don't have much context on this function or how DBConnection handles exceptions "from within" .... I think it's safer for now to keep the previous behavior of not raising and returning an error triplet. I tried to think of various scenarios in #298 (comment) and right now I don't see anything useful being lost.

else
:ok -> {:ok, query}
end
end

Expand Down
Loading
Loading