Re: DBD::Oracle, Support binding of integers so they are returned as IVs
Tim Bunce wrote:
On Tue, Oct 27, 2009 at 02:54:43PM +, Martin Evans wrote:
The next question is whether overflowing to an NV should be an error.
I'm thinking we could adopt these semantics for bind_col types:
SQL_INTEGER IV or UV via sv_2iv(sv) with error on overflow
this would be ideal.
SQL_DOUBLE NV via sv_2nv(sv)
SQL_NUMERIC IV else UV else NV via grok_number() with no error
I could sketch out the logic for those cases if you'd be happy to polish
up and test.
I would be happy to do that.
I finally got around to working on this. Here's a first rough draft
(which a bunch of issues) I thought I'd post here for discussion.
I've implemented it as a hook in the DBIS structure so drivers can call
it directly.
I've added the idea of optionally discarding the string buffer (which
would save space when storing many rows but waste time if just working
row-at-a-time). For now I've triggered that based on the sql_type but
that feels like a hack we'd regret later. A better approach might be an
attribute to bind_col:
$sth-bind_col(..., { MinMemory = 1 });
$sth-fetchall_...();
This code doesn't raise any errors or produce any warnings (directly),
it just returns a status that the driver should check if it wants to
implement the SQL_INTEGER == error on overflow semantics, which it
should if we agree that's what we're going to adopt.
Tim.
Index: DBI.xs
===
--- DBI.xs(revision 13466)
+++ DBI.xs(working copy)
@@ -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 post_fetch_sv _((pTHX_ SV *h, imp_xxh_t *imp_xxh, 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));
@@ -439,6 +440,7 @@
DBIS-set_err_sv = set_err_sv;
DBIS-set_err_char= set_err_char;
DBIS-bind_col= dbih_sth_bind_col;
+DBIS-post_fetch_sv = post_fetch_sv;
/* Remember the last handle used. BEWARE! Sneaky stuff here!*/
@@ -1714,6 +1718,94 @@
}
+/* 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:
+ * -1: sv is undef or doesn't
+ * 0: sv couldn't be converted to requested (strict) type
+ * 1: sv was handled without a problem
+ */
+int
+post_fetch_sv(pTHX_ SV *h, imp_xxh_t *imp_xxh, SV *sv, int sql_type, U32
flags, void *v)
+{
+int discard_pv = 0;
+
+/* do nothing for undef (NULL) or non-string values */
+if (!sv || !SvPOK(sv))
+return -1;
+
+switch(sql_type) {
+
+/* caller would like IV (but may get UV or NV) */
+/* will warn if not numeric. return 0 on overflow */
+case SQL_SMALLINT:
+discard_pv = 1;
+case SQL_INTEGER:
+sv_2iv(sv); /* is liberal, may return SvIV, SvUV, or SvNV */
+if (SvNOK(sv)) { /* suspicious */
+NV nv = SvNV(sv);
+/* ignore NV set just to preserve digits after the decimal place
*/
+/* just complain if the value won't fit in an IV or NV */
+if (nv UV_MAX || nv IV_MIN)
+return 0;
+}
+break;
+
+/* caller would like SvNOK/SvIOK true if the value is a number */
+/* will warn if not numeric */
+case SQL_FLOAT:
+discard_pv = 1;
+case SQL_DOUBLE:
+sv_2nv(sv);
+break;
+
+/* caller would like IV else UV else NV */
+/* else no error and sv is untouched */
+case SQL_NUMERIC:
+discard_pv = 1;
+case SQL_DECIMAL: {
+UV uv;
+/* based on the code in perl's toke.c */
+int flags = grok_number(SvPVX(sv), SvCUR(sv), uv);
+
+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);
+}
+break;
+
+#if 0 /* XXX future possibilities */
+case SQL_BIGINT:/* use Math::BigInt if too large for IV/UV */
+#endif
+
Re: DBD::Oracle, Support binding of integers so they are returned as IVs
On Mon, Nov 09, 2009 at 02:43:28PM +, Martin Evans wrote: I'm presuming you missed a bit off your patch: Yes, sorry, I was just throwing it out there for discussion of the general approach/principles. None so far. Tim.
Re: DBD::Oracle, Support binding of integers so they are returned as IVs
On Mon, Nov 09, 2009 at 05:05:11PM +, Martin Evans wrote:
Martin Evans wrote:
+/* 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:
+ * -1: sv is undef or doesn't
+ * 0: sv couldn't be converted to requested (strict) type
+ * 1: sv was handled without a problem
+ */
+int
+post_fetch_sv(pTHX_ SV *h, imp_xxh_t *imp_xxh, SV *sv, int sql_type, U32
flags, void *v)
+{
+int discard_pv = 0;
+
+/* do nothing for undef (NULL) or non-string values */
+if (!sv || !SvPOK(sv))
+return -1;
+
+switch(sql_type) {
+
+/* caller would like IV (but may get UV or NV) */
+/* will warn if not numeric. return 0 on overflow */
+case SQL_SMALLINT:
+discard_pv = 1;
+case SQL_INTEGER:
+sv_2iv(sv); /* is liberal, may return SvIV, SvUV, or SvNV */
+if (SvNOK(sv)) { /* suspicious */
+NV nv = SvNV(sv);
+/* ignore NV set just to preserve digits after the decimal
place */
+/* just complain if the value won't fit in an IV or NV */
+if (nv UV_MAX || nv IV_MIN)
+return 0;
+}
+break;
+
+/* caller would like SvNOK/SvIOK true if the value is a number */
+/* will warn if not numeric */
+case SQL_FLOAT:
+discard_pv = 1;
+case SQL_DOUBLE:
+sv_2nv(sv);
+break;
+
+/* caller would like IV else UV else NV */
+/* else no error and sv is untouched */
+case SQL_NUMERIC:
+discard_pv = 1;
+case SQL_DECIMAL: {
+UV uv;
+/* based on the code in perl's toke.c */
+int flags = grok_number(SvPVX(sv), SvCUR(sv), uv);
+
+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);
+}
+break;
+
+#if 0 /* XXX future possibilities */
+case SQL_BIGINT:/* use Math::BigInt if too large for IV/UV */
+#endif
+default:
+return 0; /* value unchanged */
+}
+
+if (discard_pv/* caller wants string buffer discarded */
+ SvNIOK(sv) /* we set a numeric value */
+ SvPVX(sv) SvLEN(sv) /* we have a buffer to discard */
+) {
+Safefree(SvPVX(sv));
+SvPVX(sv) = NULL;
+SvPOK_off(sv);
+}
+return 1;
+}
There was an omission in my addition to Tim's example as I forgot to
change DBISTATE_VERSION.
Thanks. Though that's less important than it was now there's also
DBIXS_REVISION (in dbixs_rev.h) that automatically tracks the svn
revsion number.
I've implemented this as it stands in DBD::Oracle and it seems to work
out ok and certainly where I was wanting to go (and further).
Ok.
My own feeling is that if someone asks for something to be bound as an
SQL_INTEGER and it cannot due to over/under flow this should be an error
and that is how I've implemented it.
The return value of post_fetch_sv() is meant to allow drivers to
report an error.
I thought about making post_fetch_sv() itself call DBIh_SET_ERR_* to
report an error but opted to avoid that because, to generate a good
error more info would need to be passed, like the column number.
On the other hand, if post_fetch_sv() doesn't do it then there's a
greater risk of inconsistency between the drivers.
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.
That certainly feels better than overloading SQL_INTEGER vs SQL_NUMERIC
to achieve the same effect!
However, I have
a very small concern for people who might have been binding columns with
a type but no destination SV but their DBD did nothing about it (which I
believe is all DBDs up to now). For me, I didn't leave that code in and
just documented it as:
# I was hoping the following would work (according to DBI, it
# might) to ensure the a, b and c
# columns are returned as integers instead of strings saving
# us from having to add 0 to them below. It does not with
# DBD::Oracle.
# NOTE: you don't have to pass a var into bind_col to receive
# the column data as it works on the underlying column and not
#
