I'm sorry for the delay in replying Martin.
On Tue, Nov 10, 2009 at 03:24:33PM +0000, Martin Evans wrote:
>
> >> Perhaps it could have been one of those informationals as the sv is
> >> unchanged and still usable but it is not in the requested format so
> >> I'd class that an error.
> >
> > Perhaps we should have $sth->bind_col(..., { LooselyTyped => 1 });
> > to allow for those who don't want an error if the type doesn't fit.
>
> I'm happy with that.
>
> > That certainly feels better than overloading SQL_INTEGER vs SQL_NUMERIC
> > to achieve the same effect!
>
> agreed.
> > We could make it default to issuing a warning on overflow, and have
> > attributes to specify either an error or ignore.
>
> We could but I think most people would be happy with error or specifying
> LooselyTyped. You either know you need LooselyTyped or not and if not
> you can leave it off and if it errors then your data was not as you
> thought and have to decide if your data is wrong or you need
> LooselyTyped. I'd be concerned a warning might just get in the way.
>
> >> I think a MinMemory attribute would be ok but I'd use it as in most of
> >> my cases I am retrieving the whole result-set in one go and it can be
> >> very large. How would post_fetch_sv know this attribute?
> >
> > Via the flags argument.
>
> As it turns out I /need/ MinMemory or SvPOKp(sv) returns true and that
> ends up being a string again in JSON::XS.
> > After mulling it over some more, and looking at ODBC's SQLBindCol (which
> > takes a C type, not an SQL type) I've decided to err on the simple side.
> > I've appended a patch for review.
>
> <new patch snipped>
>
> There were a few small issues with the latest patch but it was obvious
> what you meant so I have rectified them and made changes to my test
> DBD::Oracle. This works rather nicely now.
>
> At this stage I have the following changes and comments:
>
> o patch as you supplied with minor corrections in DBI
>
> ) {
> Safefree(SvPVX(sv));
> SvPVX(sv) = Nullch; /* changed from NULL to Nullch */
> SvLEN(sv) = 0; /* <--- added this as it aborts without it */
> SvPOK_off(sv);
> }
Fixed. Thanks.
> o added dbd_st_bind_col to DDB::Oracle
> stores the requested type in fbh
> handles LooselyTyped and MinMemory and saves in fbh
> defaults to DBIstcf_STRICT if LooselyTyped not specified
>
> Comment: I thought DBIstcf_STRICT was a good default but others may not.
> Comment: I was not so keen on the attribute being LooselyTyped and the
> macro being DBIstcf_STRICT as they have opposite meanings but I'm not
> really bothered as LooselyTyped is all that is really going to be visible.
I'm mostly settled on DBIstcf_STRICT _not_ being the default.
So 'loosely typed' will be the default and people who want an error
if the type can't be cast safely will need to use StrictlyTyped => 1.
I'm (slightly) open to persuasion, but that seems to fit the general
philosophy of perl and the DBI better. It also makes transition easier
for anyone who happens to have code that calls bind_column with an SQL type.
> Comment: DBIstcf_STRICT and DBIstcf_DISCARD_PV macros are not
> available as symbols to a DBD at present so I'm using 1 and 2 in
> DBD::Oracle. I wasn't sure where they'd go.
I've added them to DBIXS.h now.
I'm undecided about the name of the attribute to indicate use of
DBIstcf_DISCARD_PV. MinMemory or DiscardString or ...?
I'm leaning more towards DiscardString => 1 as it directly describes
what it does rather than a side-effect.
> o oci8.c and dbd_st_fetch modified to call DBI's sql_type_cast_svpv and
> errors when 0 (no strict conversion, with "over/under flow converting
> column N to type N") and -2 (sql type unhandled, with "unsupported bind
> type N for column N"). I don't need to think about undef in
> DBD::Oracle's case as it is handled separately.
Ok.
> o test code using all this which demonstrates binding with a type works
> and LoosleyTyped and MinMemory do as they should. Some of this will be a
> little awkward to turn into a test that works for everyone as it needs
> Devel::Peek.
Or JSON::XS :) Umm, or perhaps DBI::neat?
> What I haven't done or would appreciate feedback on:
>
> o My test DBD::Oracle currently assumes DBI has sql_type_cast_svpv. What
> is the usual way of deciding this - checking DBI version?
I've bumped DBISTATE_VERSION from 94 to 95.
(Also DBIXS_REVISION > 13590 would work.)
> o any documentation for DBI::DBD although I am happy to do so when you
> are happy we have reached a consensus.
Thank you!
> o any documentation for DBI - as above.
Thank you!
> o I wouldn't mind someone else looking over the changes as although I
> currently maintain DBD::ODBC I do not consider myself a DBI internals of
> XS expert.
Post a diff and I'll review it for you. The code you appended previously
looks ok.
> o Although my primary impetus for doing this is I need it in DBD::Oracle
> I would plan to do something similar in DBD::ODBC once this stuff is
> committed.
Great.
> o I do not have a commit bit on DBD::Oracle so will pass the DBD::Oracle
> changes to John when ready.
I'd strongly recommend John to give you a commit bit! :)
> Here are the changes so far:
You've resent a diff against the original rather than a diff of your
extra changes. I can't do much with that easily. I assume the only
significant change was adding SvLEN(sv) = 0; If there's more then let me
know (as a diff of what's changed after applying my earlier diff :)
> and DBD::Oracle (sorry about the tabbling, it seems oci.8.c has some
> very perculiar tabbling):
[Probably tabstop=4. I used to use that before I finally realized that
tab _characters_ are at 8 column intervals and trying to pretend otherwise
caused minor but widely dispursed annoyance for myself and others.
Now my tab _key_ still indents by 4 but it uses spaces to do so.
META: This is just background info, not an invitation to anyone to open
a discussion on this topic :-]
I've made some further changes:
- Renamed DBIstcf_DISCARD_PV to DBIstcf_DISCARD_STRING
- DBIstcf_DISCARD_STRING only works if the cast worked ok
- No longer pass h and imp_xxh to sql_type_cast_svpv
- Added a perl API via DBI::sql_type_cast($foo, SQL_INTEGER, $flags)
- Minor return status change to indicate status of cast even if
DBIstcf_STRICT was not used.
and committed it:
http://svn.perl.org/viewvc/modules?view=revision&revision=13591
Tim.
Index: DBI.xs
===================================================================
--- DBI.xs (revision 13590)
+++ DBI.xs (revision 13591)
@@ -78,6 +78,7 @@
static int set_err_char _((SV *h, imp_xxh_t *imp_xxh, const char *err_c, IV err_i, const char *errstr, const char *state, const char *method));
static int set_err_sv _((SV *h, imp_xxh_t *imp_xxh, SV *err, SV *errstr, SV *state, SV *method));
static int quote_type _((int sql_type, int p, int s, int *base_type, void *v));
+static int sql_type_cast_svpv _((pTHX_ SV *sv, int sql_type, U32 flags, void *v));
static I32 dbi_hash _((const char *string, long i));
static void dbih_dumphandle _((pTHX_ SV *h, const char *msg, int level));
static int dbih_dumpcom _((pTHX_ imp_xxh_t *imp_xxh, const char *msg, int level));
@@ -434,11 +435,12 @@
DBIS->get_fbav = dbih_get_fbav;
DBIS->make_fdsv = dbih_make_fdsv;
DBIS->neat_svpv = neatsvpv;
- DBIS->bind_as_num = quote_type;
+ DBIS->bind_as_num = quote_type; /* XXX deprecated */
DBIS->hash = dbi_hash;
DBIS->set_err_sv = set_err_sv;
DBIS->set_err_char= set_err_char;
DBIS->bind_col = dbih_sth_bind_col;
+ DBIS->sql_type_cast_svpv = sql_type_cast_svpv;
/* Remember the last handle used. BEWARE! Sneaky stuff here! */
@@ -1696,6 +1698,8 @@
(void)s;
(void)t;
(void)v;
+ /* looks like it's never been used, and doesn't make much sense anyway */
+ warn("Use of DBI internal bind_as_num/quote_type function is deprecated");
switch(sql_type) {
case SQL_INTEGER:
case SQL_SMALLINT:
@@ -1714,6 +1718,107 @@
}
+/* Convert a simple string representation of a value into a more specific
+ * perl type based on an sql_type value.
+ * The semantics of SQL standard TYPE values are interpreted _very_ loosely
+ * on the basis of "be liberal in what you accept and let's throw in some
+ * extra semantics while we're here" :)
+ * Returns:
+ * -2: sql_type isn't handled, value unchanged
+ * -1: sv is undef, value unchanged
+ * 0: sv couldn't be cast cleanly and DBIstcf_STRICT was used
+ * 1: sv couldn't be cast cleanly and DBIstcf_STRICT was not used
+ * 2: sv was cast ok
+ */
+
+int
+sql_type_cast_svpv(pTHX_ SV *sv, int sql_type, U32 flags, void *v)
+{
+ int cast_ok = 0;
+ int grok_flags;
+ UV uv;
+
+ /* do nothing for undef (NULL) or non-string values */
+ if (!sv || !SvOK(sv))
+ return -1;
+
+ switch(sql_type) {
+
+ default:
+ return -2; /* not a recognised SQL TYPE, value unchanged */
+
+ case SQL_INTEGER:
+ /* sv_2iv is liberal, may return SvIV, SvUV, or SvNV */
+ sv_2iv(sv);
+ /* SvNOK will be set if value is out of range for IV/UV.
+ * SvIOK should be set but won't if sv is not numeric (in which
+ * case perl would have warn'd already if -w or warnings are in effect)
+ */
+ cast_ok = (SvIOK(sv) && !SvNOK(sv));
+ break;
+
+ case SQL_DOUBLE:
+ sv_2nv(sv);
+ /* SvNOK should be set but won't if sv is not numeric (in which
+ * case perl would have warn'd already if -w or warnings are in effect)
+ */
+ cast_ok = SvNOK(sv);
+ break;
+
+ /* caller would like IV else UV else NV */
+ /* else no error and sv is untouched */
+ case SQL_NUMERIC:
+ /* based on the code in perl's toke.c */
+ uv = 0;
+ grok_flags = grok_number(SvPVX(sv), SvCUR(sv), &uv);
+ cast_ok = 1;
+ if (flags == IS_NUMBER_IN_UV) { /* +ve int */
+ if (uv <= IV_MAX) /* prefer IV over UV */
+ sv_2iv(sv);
+ else sv_2uv(sv);
+ }
+ else if (flags == (IS_NUMBER_IN_UV | IS_NUMBER_NEG)
+ && uv <= IV_MAX
+ ) {
+ sv_2iv(sv);
+ }
+ else if (flags) { /* is numeric */
+ sv_2nv(sv);
+ }
+ else if (flags & DBIstcf_STRICT)
+ cast_ok = 0;
+ break;
+
+#if 0 /* XXX future possibilities */
+ case SQL_BIGINT: /* use Math::BigInt if too large for IV/UV */
+#endif
+ }
+
+ if (cast_ok) {
+
+ if (flags & DBIstcf_DISCARD_STRING
+ && SvNIOK(sv) /* we set a numeric value */
+ && SvPVX(sv) /* we have a buffer to discard */
+ ) {
+ SvOOK_off(sv);
+ if (SvLEN(sv)) {
+ Safefree(SvPVX(sv));
+ SvLEN(sv) = 0;
+ }
+ SvPVX(sv) = NULL;
+ SvPOK_off(sv);
+ }
+ }
+
+ if (cast_ok)
+ return 2;
+ else if (flags & DBIstcf_STRICT)
+ return 0;
+ else return 1;
+}
+
+
+
/* --- Generic Handle Attributes (for all handle types) --- */
static int
@@ -4007,7 +4112,7 @@
SQL_ALL_TYPES = SQL_ALL_TYPES
SQL_ARRAY = SQL_ARRAY
SQL_ARRAY_LOCATOR = SQL_ARRAY_LOCATOR
- SQL_BIGINT = SQL_BIGINT
+ SQL_BIGINT = SQL_BIGINT
SQL_BINARY = SQL_BINARY
SQL_BIT = SQL_BIT
SQL_BLOB = SQL_BLOB
@@ -4081,6 +4186,8 @@
DBIpp_st_qq = DBIpp_st_qq
DBIpp_st_bs = DBIpp_st_bs
DBIpp_st_XX = DBIpp_st_XX
+ DBIstcf_DISCARD_STRING = DBIstcf_DISCARD_STRING
+ DBIstcf_STRICT = DBIstcf_STRICT
CODE:
RETVAL = ix;
OUTPUT:
@@ -4438,10 +4545,18 @@
RETVAL
+int
+sql_type_cast(sv, sql_type, flags=0)
+ SV * sv
+ int sql_type
+ U32 flags
+ CODE:
+ RETVAL = sql_type_cast_svpv(aTHX_ sv, sql_type, flags, 0);
+ OUTPUT:
+ RETVAL
-
MODULE = DBI PACKAGE = DBI::var
void
Index: DBIXS.h
===================================================================
--- DBIXS.h (revision 13590)
+++ DBIXS.h (revision 13591)
@@ -342,7 +342,11 @@
#define neatsvpv(sv,len) (DBIS->neat_svpv(sv,len))
#endif
+/* --- For sql_type_cast_svpv() --- */
+#define DBIstcf_DISCARD_STRING 0x0001
+#define DBIstcf_STRICT 0x0002
+
/* --- Implementors Private Data Support --- */
#define D_impdata(name,type,h) type *name = (type*)(DBIh_COM(h))
@@ -392,7 +396,7 @@
struct dbistate_st {
-#define DBISTATE_VERSION 94 /* Must change whenever dbistate_t does */
+#define DBISTATE_VERSION 95 /* Must change whenever dbistate_t does */
/* this must be the first member in structure */
void (*check_version) _((const char *name,
@@ -417,7 +421,7 @@
SV * (*get_attr_k) _((SV *h, SV *keysv, int dbikey));
AV * (*get_fbav) _((imp_sth_t *imp_sth));
SV * (*make_fdsv) _((SV *sth, const char *imp_class, STRLEN imp_size, const char *col_name));
- int (*bind_as_num) _((int sql_type, int p, int s, int *t, void *v));
+ int (*bind_as_num) _((int sql_type, int p, int s, int *t, void *v)); /* XXX deprecated */
I32 (*hash) _((const char *string, long i));
SV * (*preparse) _((SV *sth, char *statement, IV ps_return, IV ps_accept, void *foo));
@@ -430,11 +434,13 @@
int (*set_err_char) _((SV *h, imp_xxh_t *imp_xxh, const char *err, IV err_i, const char *errstr, const char *state, const char *method));
int (*bind_col) _((SV *sth, SV *col, SV *ref, SV *attribs));
- IO *logfp_ref; /* DAA keep ptr to filehandle for refcounting */
+ IO *logfp_ref; /* keep ptr to filehandle for refcounting */
+ int (*sql_type_cast_svpv) _((pTHX_ SV *sv, int sql_type, U32 flags, void *v));
+
/* WARNING: Only add new structure members here, and reduce pad2 to keep */
/* the memory footprint exactly the same */
- void *pad2[4];
+ void *pad2[3];
};
/* macros for backwards compatibility */
Index: Changes
===================================================================
--- Changes (revision 13590)
+++ Changes (revision 13591)
@@ -8,13 +8,12 @@
=head2 Changes in DBI 1.611 (svn rXXX)
-XXX needs to be redone to convert ReadOnly to an internal flag:
-
- Fixes DBI->trace(0, *STDERR); (H.Merijn Brand)
+ Fixed DBI->trace(0, *STDERR); (H.Merijn Brand)
which tried to open a file named "*main::STDERR" in perl-5.10.x
Bumped required perl version to 5.8.1 (as announced in DBI 1.607)
+XXX needs to be redone to convert ReadOnly to an internal flag:
Changed "Issuing rollback() due to DESTROY without explicit disconnect"
warning to not be issued if ReadOnly set for that dbh.
Updated dbipport.h to Devel::PPPort 3.19 (H.Merijn Brand)
@@ -24,8 +23,17 @@
Added ChildCallbacks => { ... } to Callbacks as a way to
specify Callbacks for child handles.
With tests and docs thanks to David E. Wheeler.
-XXX awaiting docs
+XXX awaiting docs for ChildCallbacks from David.
+ Added DBI::sql_type_cast($value, $type, $flags) to cast a string value
+ to an SQL type. e.g. SQL_INTEGER effectively does $value += 0;
+ Has other options plus an internal interface for drivers.
+
+ Added specification of type casting behaviour for bind_column()
+ based on DBI::sql_type_cast() and two new bind_column attributes
+ StrictlyTyped and DiscardString. Thanks to Martin Evans.
+XXX awaiting docs for sql_type_cast & bind_column from Martin Evans.
+
=head2 Changes in DBI 1.609 (svn r12816) 8th June 2009
Fixes to DBD::File (H.Merijn Brand)
Index: dbixs_rev.h
===================================================================
--- dbixs_rev.h (revision 13590)
+++ dbixs_rev.h (revision 13591)
@@ -1,4 +1,3 @@
-/* Mon Nov 2 22:44:58 2009 */
-/* Mixed revision working copy (13455M:13465) */
+/* Tue Nov 24 12:53:00 2009 */
/* Code modified since last checkin */
-#define DBIXS_REVISION 13455
+#define DBIXS_REVISION 13590
Index: DBI.pm
===================================================================
--- DBI.pm (revision 13590)
+++ DBI.pm (revision 13591)
@@ -223,6 +223,8 @@
SQL_INTERVAL_HOUR_TO_MINUTE
SQL_INTERVAL_HOUR_TO_SECOND
SQL_INTERVAL_MINUTE_TO_SECOND
+ DBIstcf_DISCARD_STRING
+ DBIstcf_STRICT
) ],
sql_cursor_types => [ qw(
SQL_CURSOR_FORWARD_ONLY
@@ -233,7 +235,7 @@
) ], # for ODBC cursor types
utils => [ qw(
neat neat_list $neat_maxlen dump_results looks_like_number
- data_string_diff data_string_desc data_diff
+ data_string_diff data_string_desc data_diff sql_type_cast
) ],
profile => [ qw(
dbi_profile dbi_profile_merge dbi_profile_merge_nodes dbi_time
