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" <yum...@gmail.com> 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 DBD-SQLite@lists.scsys.co.uk http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbd-sqlite