Description in patch.
This change is a bit more questionable.
Pros:
1) Saves some memory (but not much);
2) Removes outdated and confusing comment;
3) IMO, makes code flow more natural;
Cons:
1) Does not fix any "real" bug;
2) Changes behavior a bit - with patch applied, binding errors are raised by
$sth->bind_param, before this patch errors was raised by $sth->execute.
From 5ba5fa18dbaa28aea32a6aaa384fa5ed9570daf9 Mon Sep 17 00:00:00 2001
From: "Yuriy M. Kaminskiy" <[email protected]>
Date: Mon, 17 Oct 2011 08:45:36 +0400
Subject: [PATCH] Move parameter binding back to (sqlite|dbd)_bind_ph
Since DBD::SQLite 1.14 and transition to sqlite3_prepare_v2, we no
longer need to delay binding parameters till (dbd|sqlite)_st_execute.
WARNING: change in behavior: as binding happens now in bind_param,
all other possible sqlite binding errors are detected in bind_param
and no longer delayed till execute.
Also, "datatype mismatch" error (introduced in 1.34_02) is now detected
right inside bind_param, and no longer delayed till execute.
Updated t/rt_67581_bind_params_mismatch.t accordingly.
---
dbdimp.c | 152 ++++++++++++++++++-------------------
dbdimp.h | 2 +-
t/rt_67581_bind_params_mismatch.t | 37 ++++++---
3 files changed, 98 insertions(+), 93 deletions(-)
diff --git a/dbdimp.c b/dbdimp.c
index d2092c8..007638b 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -634,8 +634,8 @@ sqlite_st_prepare(SV *sth, imp_sth_t *imp_sth, char *statement, SV *attribs)
sqlite_trace(sth, imp_sth, 3, form("prepare statement: %s", statement));
imp_sth->nrow = -1;
imp_sth->retval = SQLITE_OK;
- imp_sth->params = newAV();
imp_sth->col_types = newAV();
+ imp_sth->param_types = newAV();
croak_if_db_is_null();
@@ -706,73 +706,6 @@ sqlite_st_execute(SV *sth, imp_sth_t *imp_sth)
}
}
- for (i = 0; i < num_params; i++) {
- SV **pvalue = av_fetch(imp_sth->params, 2*i, 0);
- SV **sql_type_sv = av_fetch(imp_sth->params, 2*i+1, 0);
- SV *value = pvalue ? *pvalue : &PL_sv_undef;
- int sql_type = sqlite_type_from_odbc_type(sql_type_sv ? SvIV(*sql_type_sv) : SQL_UNKNOWN_TYPE);
-
- sqlite_trace(sth, imp_sth, 4, form("bind %d type %d as %s", i, sql_type, SvPV_nolen_undef_ok(value)));
-
- if (!SvOK(value)) {
- sqlite_trace(sth, imp_sth, 5, "binding null");
- rc = sqlite3_bind_null(imp_sth->stmt, i+1);
- }
- else if (sql_type == SQLITE_BLOB) {
- STRLEN len;
- char * data = SvPVbyte(value, len);
- rc = sqlite3_bind_blob(imp_sth->stmt, i+1, data, len, SQLITE_TRANSIENT);
- }
- else {
- STRLEN len;
- const char *data;
- int numtype = 0;
-
- if (imp_dbh->unicode) {
- sv_utf8_upgrade(value);
- }
- data = SvPV(value, len);
-
- /*
- * XXX: For backward compatibility, it'd be better to
- * accept a value like " 4" as an integer for an integer
- * type column (see t/19_bindparam.t), at least when
- * we explicitly specify its type. However, we should
- * keep spaces when we just guess.
- */
- if (imp_dbh->see_if_its_a_number) {
- numtype = sqlite_is_number(aTHX_ data, TRUE);
- }
- else if (sql_type == SQLITE_INTEGER || sql_type == SQLITE_FLOAT) {
- numtype = sqlite_is_number(aTHX_ data, FALSE);
- }
-
- if (numtype == 1) {
-#if defined(USE_64_BIT_INT)
- rc = sqlite3_bind_int64(imp_sth->stmt, i+1, atoi(data));
-#else
- rc = sqlite3_bind_int(imp_sth->stmt, i+1, atoi(data));
-#endif
- }
- else if (numtype == 2 && sql_type != SQLITE_INTEGER) {
- rc = sqlite3_bind_double(imp_sth->stmt, i+1, atof(data));
- }
- else {
- if (sql_type == SQLITE_INTEGER || sql_type == SQLITE_FLOAT) {
- sqlite_error(sth, -2, form("datatype mismatch: bind %d type %d as %s", i, sql_type, SvPV_nolen_undef_ok(value)));
-
- return -2; /* -> undef in SQLite.xsi */
- }
- rc = sqlite3_bind_text(imp_sth->stmt, i+1, data, len, SQLITE_TRANSIENT);
- }
- }
-
- if (rc != SQLITE_OK) {
- sqlite_error(sth, rc, sqlite3_errmsg(imp_dbh->db));
- return -4; /* -> undef in SQLite.xsi */
- }
- }
-
if (sqlite3_get_autocommit(imp_dbh->db)) {
/* COMPAT: sqlite3_sql is only available for 3006000 or newer */
const char *sql = sqlite3_sql(imp_sth->stmt);
@@ -1075,7 +1008,7 @@ sqlite_st_destroy(SV *sth, imp_sth_t *imp_sth)
}
}
}
- SvREFCNT_dec((SV*)imp_sth->params);
+ SvREFCNT_dec((SV*)imp_sth->param_types);
SvREFCNT_dec((SV*)imp_sth->col_types);
DBIc_IMPSET_off(imp_sth);
}
@@ -1192,11 +1125,6 @@ sqlite_st_FETCH_attrib(SV *sth, imp_sth_t *imp_sth, SV *keysv)
return retsv;
}
-/* bind parameter
- * NB: We store the params instead of bind immediately because
- * we might need to re-create the imp_sth->stmt (see top of execute() function)
- * and so we can't lose these params
- */
int
sqlite_bind_ph(SV *sth, imp_sth_t *imp_sth,
SV *param, SV *value, IV sql_type, SV *attribs,
@@ -1204,6 +1132,9 @@ sqlite_bind_ph(SV *sth, imp_sth_t *imp_sth,
{
dTHX;
int pos;
+ int sqlite_type;
+ int rc;
+ D_imp_dbh_from_sth;
croak_if_stmt_is_null();
@@ -1222,7 +1153,6 @@ sqlite_bind_ph(SV *sth, imp_sth_t *imp_sth,
sqlite_error(sth, -2, form("Unknown named parameter: %s", paramstring));
return FALSE; /* -> &sv_no in SQLite.xsi */
}
- pos = 2 * (pos - 1);
}
else {
sqlite_error(sth, -2, "<param> could not be coerced to a C string");
@@ -1230,12 +1160,76 @@ sqlite_bind_ph(SV *sth, imp_sth_t *imp_sth,
}
}
else {
- pos = 2 * (SvIV(param) - 1);
+ pos = SvIV(param);
}
- sqlite_trace(sth, imp_sth, 3, form("bind into 0x%p: %"IVdf" => %s (%"IVdf") pos %d", imp_sth->params, SvIV(param), SvPV_nolen_undef_ok(value), sql_type, pos));
- av_store(imp_sth->params, pos, SvREFCNT_inc(value));
if (sql_type) {
- av_store(imp_sth->params, pos+1, newSViv(sql_type));
+ av_store(imp_sth->param_types, pos, newSViv(sql_type));
+ }
+ else {
+ SV **sql_type_sv = av_fetch(imp_sth->param_types, pos, 0);
+ sql_type = sql_type_sv ? SvIV(*sql_type_sv) : SQL_UNKNOWN_TYPE;
+ }
+
+ sqlite_type = sqlite_type_from_odbc_type(sql_type);
+
+ sqlite_trace(sth, imp_sth, 3, form("bind into 0x%p: %"IVdf" => %s (sql: %"IVdf", sqlite: %d) pos %d", imp_sth, pos, SvPV_nolen_undef_ok(value), sql_type, sqlite_type, pos));
+
+ if (!SvOK(value)) {
+ sqlite_trace(sth, imp_sth, 5, "binding null");
+ rc = sqlite3_bind_null(imp_sth->stmt, pos);
+ }
+ else if (sqlite_type == SQLITE_BLOB) {
+ STRLEN len;
+ char * data = SvPVbyte(value, len);
+ rc = sqlite3_bind_blob(imp_sth->stmt, pos, data, len, SQLITE_TRANSIENT);
+ }
+ else {
+ STRLEN len;
+ const char *data;
+ int numtype = 0;
+
+ if (imp_dbh->unicode) {
+ sv_utf8_upgrade(value);
+ }
+ data = SvPV(value, len);
+
+ /*
+ * XXX: For backward compatibility, it'd be better to
+ * accept a value like " 4" as an integer for an integer
+ * type column (see t/19_bindparam.t), at least when
+ * we explicitly specify its type. However, we should
+ * keep spaces when we just guess.
+ */
+ if (imp_dbh->see_if_its_a_number) {
+ numtype = sqlite_is_number(aTHX_ data, TRUE);
+ }
+ else if (sqlite_type == SQLITE_INTEGER || sqlite_type == SQLITE_FLOAT) {
+ numtype = sqlite_is_number(aTHX_ data, FALSE);
+ }
+
+ if (numtype == 1) {
+#if defined(USE_64_BIT_INT)
+ rc = sqlite3_bind_int64(imp_sth->stmt, pos, atoi(data));
+#else
+ rc = sqlite3_bind_int(imp_sth->stmt, pos, atoi(data));
+#endif
+ }
+ else if (numtype == 2 && sqlite_type != SQLITE_INTEGER) {
+ rc = sqlite3_bind_double(imp_sth->stmt, pos, atof(data));
+ }
+ else {
+ if (sqlite_type == SQLITE_INTEGER || sqlite_type == SQLITE_FLOAT) {
+ sqlite_error(sth, -2, form("datatype mismatch: bind %d type %d as %s", pos, sqlite_type, SvPV_nolen_undef_ok(value)));
+
+ return FALSE; /* -> undef in SQLite.xsi */
+ }
+ rc = sqlite3_bind_text(imp_sth->stmt, pos, data, len, SQLITE_TRANSIENT);
+ }
+ }
+
+ if (rc != SQLITE_OK) {
+ sqlite_error(sth, rc, sqlite3_errmsg(imp_dbh->db));
+ return FALSE; /* -> undef in SQLite.xsi */
}
return TRUE;
diff --git a/dbdimp.h b/dbdimp.h
index 75aae83..631615c 100644
--- a/dbdimp.h
+++ b/dbdimp.h
@@ -49,7 +49,7 @@ struct imp_sth_st {
*/
int retval;
int nrow;
- AV *params;
+ AV *param_types;
AV *col_types;
const char *unprepared_statements;
};
diff --git a/t/rt_67581_bind_params_mismatch.t b/t/rt_67581_bind_params_mismatch.t
index c0d4b52..32eab08 100644
--- a/t/rt_67581_bind_params_mismatch.t
+++ b/t/rt_67581_bind_params_mismatch.t
@@ -23,8 +23,10 @@ for my $has_pk (0..1) {
{
my $sth = $dbh->prepare('insert into foo values (?, ?)');
$sth->bind_param(1, ++$id);
- $sth->bind_param(2, 1);
- my $ret = eval { $sth->execute };
+ my $ret = eval {
+ $sth->bind_param(2, 1);
+ $sth->execute
+ };
ok defined $ret, "inserted without errors";
my ($value) = $dbh->selectrow_array('select v from foo where id = ?', undef, $id);
@@ -34,8 +36,10 @@ for my $has_pk (0..1) {
{
my $sth = $dbh->prepare('insert into foo values (?, ?)');
$sth->bind_param(1, ++$id);
- $sth->bind_param(2, 1.5);
- my $ret = eval { $sth->execute };
+ my $ret = eval {
+ $sth->bind_param(2, 1.5);
+ $sth->execute
+ };
if ($has_pk) {
ok $@, "died correctly";
@@ -59,8 +63,10 @@ for my $has_pk (0..1) {
{
my $sth = $dbh->prepare('insert into foo values (?, ?)');
$sth->bind_param(1, ++$id);
- $sth->bind_param(2, 'foo'); # may seem weird, but that's sqlite
- my $ret = eval { $sth->execute };
+ my $ret = eval {
+ $sth->bind_param(2, 'foo'); # may seem weird, but that's sqlite
+ $sth->execute
+ };
if ($has_pk) {
ok $@, "died correctly";
@@ -84,8 +90,10 @@ for my $has_pk (0..1) {
{
my $sth = $dbh->prepare('insert into foo values (?, ?)');
$sth->bind_param(1, ++$id);
- $sth->bind_param(2, 3, SQL_INTEGER);
- my $ret = eval { $sth->execute };
+ my $ret = eval {
+ $sth->bind_param(2, 3, SQL_INTEGER);
+ $sth->execute
+ };
ok defined $ret, "inserted without errors";
my ($value) = $dbh->selectrow_array('select v from foo where id = ?', undef, $id);
@@ -95,8 +103,10 @@ for my $has_pk (0..1) {
{
my $sth = $dbh->prepare('insert into foo values (?, ?)');
$sth->bind_param(1, ++$id);
- $sth->bind_param(2, 3.5, SQL_INTEGER);
- my $ret = eval { $sth->execute };
+ my $ret = eval {
+ $sth->bind_param(2, 3.5, SQL_INTEGER);
+ $sth->execute;
+ };
ok $@, "died correctly";
ok !defined $ret, "returns undef";
ok $sth->errstr && $sth->errstr =~ /datatype mismatch/, "insert failed: type mismatch";
@@ -108,10 +118,11 @@ for my $has_pk (0..1) {
{
my $sth = $dbh->prepare('insert into foo values (?, ?)');
$sth->bind_param(1, ++$id);
- $sth->bind_param(2, 'qux', SQL_INTEGER);
-
# only dies if type is explicitly specified
- my $ret = eval { $sth->execute };
+ my $ret = eval {
+ $sth->bind_param(2, 'qux', SQL_INTEGER);
+ $sth->execute
+ };
ok $@, "died correctly";
ok !defined $ret, "returns undef";
ok $sth->errstr && $sth->errstr =~ /datatype mismatch/, "insert failed: type mismatch";
--
1.7.6.3
_______________________________________________
DBD-SQLite mailing list
[email protected]
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbd-sqlite
