Support binding of integers in DBD::Oracle so they are returned as IVs

I've reported Support binding of integers so they are returned as IVs at rt for DBD::Oracle.

The query I am executing returns a mix of strings and integers which are fetched with fetchall_arrayref. The returned data is then fed into JSON::XS to produce a JSON string. JSON::XS encodes strings as "string" and numbers as number (no quotes) but it detects numbers using:

if (SvPOKp (sv))
{
     STRLEN len;
     char *str = SvPV (sv, len);
     encode_ch (enc, '"');
     encode_str (enc, str, len, SvUTF8 (sv));
     encode_ch (enc, '"');
}
else if (SvNOKp (sv))
{
     // trust that perl will do the right thing w.r.t. JSON syntax.
     need (enc, NV_DIG + 32);
     Gconvert (SvNVX (sv), NV_DIG, 0, enc->cur);
     enc->cur += strlen (enc->cur);
}
else if (SvIOKp (sv))
{
     // we assume we can always read an IV as a UV and vice versa
     // we assume two's complement
     // we assume no aliasing issues in the union

My query returns 209 thousand rows and the JSON encoded data is over 21Mb. 3 of the fields are really numbers so if they are encoded as numbers we lose 2 " around each of the numbers vastly reducing the size of the encoded data. As DBD::Oracle does not support returning columns as integers I have to loop through all 209000 rows adding 0 to each field which is a number to make it an IV - this takes a lot of time - believe me, I've Devel::NYTProf'ed it.

What I want to be able to do is call bind_col(N, undef, {type => SQL_INTEGER}) as per the DBI specification and have the column returned as an integer but also as the DBI specification says, not have to actually bind to a scalar.

I'm sure this would be of general usefulness, I only cite my specific example to help explain why this would be good.

Comments

posted to dbi-dev mailing list

Some progress on binding columns as IVs

The thread on dbi-dev has expanded and there were some useful comments. Of particular interest was Greg Sabino Mullane's contribution showing the code he had changed in DBD::Pg to do something similar (which I believe was prompted by someone using JSON::XS). Unfortunately, Oracle can store some extremely large integers and I think DBD::Pg only had to deal with numbers that would fit into an IV. Also, I don't want any automatic binding of columns to IVs since a number of my columns are integers but I only want some of them as IVs. I spent an hour or so on this today and have come up with the following patch (although you should note it is no where near complete although it does appear to work):

Index: oci8.c
===================================================================
--- oci8.c      (revision 13427)
+++ oci8.c      (working copy)
@@ -3279,10 +3279,32 @@
                                                while(datalen && p[datalen - 1]==' ')
                                                        --datalen;
                                        }
-                                       sv_setpvn(sv, p, (STRLEN)datalen);
-                                       if (CSFORM_IMPLIES_UTF8(fbh->csform) ){
-                                               SvUTF8_on(sv);
-                                       }
+
+                    if ((fbh->req_type == SQLT_INT) &&
+                        ((fbh->dbtype == SQLT_INT) ||
+                         (fbh->dbtype == SQLT_NUM))){
+                        char *e;
+                        char zval[32];
+                        long val;
+
+                        memcpy(zval, p, datalen);
+                        zval[datalen] = '\0';
+                        val = strtol(zval, &e, 10);
+
+                        if ((val == LONG_MAX) || (val == LONG_MIN) ||
+                            (e && (*e != '\0'))) {
+                            oci_error(sth, imp_sth->errhp, OCI_ERROR,
+                                      "invalid number or over/under flow");
+                            return Nullav;
+                        }
+                        
+                        sv_setiv(sv, val);
+                    } else {
+                        sv_setpvn(sv, p, (STRLEN)datalen);
+                        if (CSFORM_IMPLIES_UTF8(fbh->csform) ){
+                            SvUTF8_on(sv);
+                        }
+                    }                                        
                                }
                        }
 
Index: dbdimp.c
===================================================================
--- dbdimp.c    (revision 13427)
+++ dbdimp.c    (working copy)
@@ -869,7 +869,20 @@
        return 1;
 }
 
+int dbd_st_bind_col(SV *sth, imp_sth_t *imp_sth, SV *col, SV *ref, IV type, SV *attribs) {
+    dTHX;
 
+    int field = SvIV(col);
+
+    if (field <= DBIc_NUM_FIELDS(imp_sth)) {
+        imp_sth->fbh[field-1].req_type = type;
+    }
+    
+
+    return 1;
+}
+
 int
 dbd_db_disconnect(SV *dbh, imp_dbh_t *imp_dbh)
 {
Index: dbdimp.h
===================================================================
--- dbdimp.h    (revision 13427)
+++ dbdimp.h    (working copy)
@@ -191,6 +191,8 @@
        int                     piece_lob;  /*use piecewise fetch for lobs*/
        /* Our storage space for the field data as it's fetched */
        sword           ftype;  /* external datatype we wish to get     */
+        IV            req_type;                /* type passed to bind_col */
+    
        fb_ary_t        *fb_ary ;       /* field buffer array                   */
        /* if this is an embedded object we use this */
        fbh_obj_t       *obj;
@@ -371,6 +373,7 @@
 #define dbd_st_FETCH_attrib    ora_st_FETCH_attrib
 #define dbd_describe           ora_describe
 #define dbd_bind_ph                    ora_bind_ph
+#define dbd_st_bind_col         ora_st_bind_col
 #include "ocitrace.h"
 
 /* end */
Index: Oracle.h
===================================================================
--- Oracle.h    (revision 13427)
+++ Oracle.h    (working copy)
@@ -67,6 +67,7 @@
 int     dbd_db_do _((SV *sv, char *statement));
 int     dbd_db_commit     _((SV *dbh, imp_dbh_t *imp_dbh));
 int     dbd_db_rollback   _((SV *dbh, imp_dbh_t *imp_dbh));
+int dbd_st_bind_col(SV *sth, imp_sth_t *imp_sth, SV *col, SV *ref, IV type, SV *attribs);
 int     dbd_db_disconnect _((SV *dbh, imp_dbh_t *imp_dbh));
 void dbd_db_destroy    _((SV *dbh, imp_dbh_t *imp_dbh));
 int     dbd_db_STORE_attrib _((SV *dbh, imp_dbh_t *imp_dbh, SV *keysv, SV *valuesv));

The following simple test script demonstrates it working, first with a simple table called mje with a column a integer and secondly with a function that returns a cursor on a result-set where the first 2 columns are bound as integers.

use DBI;
use JSON::XS;
use Data::Dumper;
use DBD::Oracle qw(:ora_types);

my $h = DBI->connect('dbi:Oracle:host=xxx.xxx.local;sid=devel',
                     'xxx', 'xxx');
my $s = $h->prepare('select a,a from mje');
$s->execute;
$s->bind_col(1, undef, {TYPE=> SQLT_NUM});
my $data = $s->fetchall_arrayref;
print Dumper($data);

my $js = encode_json($data);

print "$js\n";

$s = $h->prepare('BEGIN ? := pkg_xxx.xxx(?); END;');
my $cursor;
$s->bind_param_inout(1, \$cursor, 100, {ora_type => ORA_RSET});
$s->bind_param(2, 1000);
$s->execute;
$cursor->bind_col(1, undef, {TYPE => SQLT_NUM});
$cursor->bind_col(2, undef, {TYPE => SQLT_NUM});
$data = $cursor->fetchall_arrayref;
print Dumper($data);
$js = encode_json($data);
print "$js\n";

More progress binding columns as integers

For the last few days I've been working on this on and off and with Tim's help we've got quite some way.

See Re: DBD::Oracle, Support binding of integers so they are returned as IVs.

As per the discussion on dbi-dev I've changed the bind types to DBI (ODBC) types so they become SQL_INTEGER etc.

I've implemented MinMemory and LooselyTyped attributes to bind_col in DBD::Oracle. The former frees the PV associated with the SV (which is a requirement for making the returned data look like integers for JSON::XS) and the latter allows you to specify you want the column bound as type XXX but if that over/under flows it is not an error (usually it is). The default is LooselyTyped = 0 and MinMemory = 0 for now.

So now you can do:

$cursor->bind_col(1, undef, {TYPE => SQL_INTEGER, MinMemory =>1, LooselyType => 1);

and you get column 1 back as an IV unless it cannot fit in an IV in which case it will be returned as a PV and not error in this case (due to LooselyTyped => 1).

This works really well for me and now I can avoid looping through a result-set from Oracle adding 0 to columns I want as integers - this saves a lot of time. When this all settles down I'll add it to DBD::ODBC too.

Final patch submitted to dbi-dev

I've been able to snatch a few minutes here and there to work on this. I think the DBI side of things is now complete (with the exception of a small problem with doubles). sql_type_castsvpv is done and documented in DBI::DBD and the new attributes (now StrictlyTyped and DiscardString) are documented in DBI. The latest DBD::Oracle changes are below. I still need to look at why doubles do not return the right status in all cases.

Index: oci8.c
===================================================================
--- oci8.c      (revision 13624)
+++ oci8.c      (working copy)
@@ -431,7 +431,7 @@
        case OCI_ATTR_FSPRECISION OCI_ATTR_PDSCL:return "";*/
                                                  /* fs prec for datetime data types */
        case OCI_ATTR_PDPRC:                            return "OCI_ATTR_PDPRC";                         /* packed decimal format
-       case OCI_ATTR_LFPRECISION OCI_ATTR_PDPRC: return "";
+                                                                                                            case OCI_ATTR_LFPRECISION OCI_ATTR_PDPRC: return ""; */

                                                  /* fs prec for datetime data types */
        case OCI_ATTR_PARAM_COUNT:                      return "OCI_ATTR_PARAM_COUNT";       /* number of column in the select list */
        case OCI_ATTR_ROWID:                            return "OCI_ATTR_ROWID";                                     /* the rowid */
@@ -513,7 +513,7 @@
        case OCI_ATTR_COL_COUNT:                        return "OCI_ATTR_COL_COUNT";        /* columns of column array
                                                             processed so far.       */

        case OCI_ATTR_STREAM_OFFSET:            return "OCI_ATTR_STREAM_OFFSET";  /* str off of last row processed
-       case OCI_ATTR_SHARED_HEAPALLO:                          return "";    /* Shared Heap Allocation Size */

+                                                                                     case OCI_ATTR_SHARED_HEAPALLO:                            return "";*/    /* Shared Heap Allocation Size */
 
        case OCI_ATTR_SERVER_GROUP:                     return "OCI_ATTR_SERVER_GROUP";    /* server group name */
 
@@ -686,7 +686,7 @@
 
        /* Attr to allow setting of the stream version PRIOR to calling Prepare */
        case OCI_ATTR_DIRPATH_STREAM_VERSION:   return "OCI_ATTR_DIRPATH_STREAM_VERSION";      /* version of the stream
-       case OCI_ATTR_RESERVED_11:                              return "OCI_ATTR_RESERVED_11";                 /* reserved */

+                                                                                                  case OCI_ATTR_RESERVED_11:                           return "OCI_ATTR_RESERVED_11";*/                 /* reserved */
 
        case OCI_ATTR_RESERVED_12:                      return "OCI_ATTR_RESERVED_12";                 /* reserved */
        case OCI_ATTR_RESERVED_13:                      return "OCI_ATTR_RESERVED_13";                 /* reserved */
@@ -2668,7 +2668,7 @@
 
 
 
-/*static int                   /* --- Setup the row cache for this sth --- */
+static int                     /* --- Setup the row cache for this sth --- */
 sth_set_row_cache(SV *h, imp_sth_t *imp_sth, int max_cache_rows, int num_fields, int has_longs)
 {
        dTHX;
@@ -3742,10 +3742,38 @@
                                                while(datalen && p[datalen - 1]==' ')
                                                        --datalen;
                                        }
-                                       sv_setpvn(sv, p, (STRLEN)datalen);
-                                       if (CSFORM_IMPLIES_UTF8(fbh->csform) ){
-                                               SvUTF8_on(sv);
-                                       }
+                    sv_setpvn(sv, p, (STRLEN)datalen);
+#if DBISTATE_VERSION > 94
+                    /* DBIXS_REVISION > 13590 */
+                    /* If a bind type was specified we use DBI's sql_type_cast
+                       to cast it - currently only number types are handled */

+                    if (fbh->req_type != 0) {
+                        int sts;
+                        D_imp_xxh(sth);
+                        char errstr[256];
+
+                        sts = DBIc_DBISTATE(imp_sth)->sql_type_cast_svpv(
+                            aTHX_ sv, fbh->req_type, fbh->bind_flags, NULL);
+                        if (sts == 0) {
+                            sprintf(errstr,
+                                    "over/under flow converting column %d to type %ld",
+                                    i+1, fbh->req_type);
+                            oci_error(sth, imp_sth->errhp, OCI_ERROR, errstr);
+                            return Nullav;
+
+                        } else if (sts == -2) {
+                            sprintf(errstr,
+                                    "unsupported bind type %ld for column %d",
+                                    fbh->req_type, i+1);
+                            return Nullav;
+                        }
+                    } else
+#endif /* DBISTATE_VERSION > 94 */
+                    {
+                        if (CSFORM_IMPLIES_UTF8(fbh->csform) ){
+                            SvUTF8_on(sv);
+                        }
+                    }
                                }
                        }
 
Index: Changes
===================================================================
--- Changes     (revision 13624)
+++ Changes     (working copy)
@@ -13,6 +13,7 @@
   Fix for rt.cpan.org Ticket #=46246 fetching from nested cursor (returned from procedure) leads to application crash (abort) from John Scoles
   Fix for rt.cpan.org Ticket #=46016  LOBs bound with ora_field broken from RKITOVER
   Fix for bug in 58object.t when test run as externally identified user from Charles Jardine
+  Add support for the TYPE attribute on bind_col and the new DBI bind_col attributes StrictlyTyped and DiscardString from Martin J. Evans
 
 =head1 Changes in DBD-Oracle 1.23(svn rev 12724)
   Fix from rt.cpan.org ticket #=44788 bool in_lite should be char in_literal
Index: dbdimp.c
===================================================================
--- dbdimp.c    (revision 13624)
+++ dbdimp.c    (working copy)
@@ -871,7 +871,50 @@
        return 1;
 }
 
+int dbd_st_bind_col(SV *sth, imp_sth_t *imp_sth, SV *col, SV *ref, IV type, SV *attribs) {
+    dTHX;
+    int field;
 
+    if (!SvIOK(col)) {
+        croak ("Invalid column number") ;
+    }
+
+    field = SvIV(col);
+
+    if ((field < 1) || (field > DBIc_NUM_FIELDS(imp_sth))) {
+        croak("cannot bind to non-existent field %d", field);
+    }
+
+    imp_sth->fbh[field-1].req_type = type;
+    imp_sth->fbh[field-1].bind_flags = 0; /* default to none */
+
+#if DBIXS_REVISION >= 13590
+    /* DBIXS 13590 added StrictlyTyped and DiscardString attributes */
+    if (attribs) {
+        HV *attr_hash;
+        SV **attr;
+
+        if (!SvROK(attribs)) {
+            croak ("attributes is not a reference");
+        } else if (SvTYPE(SvRV(attribs)) != SVt_PVHV) {
+            croak ("attributes not a hash reference");
+        }
+        attr_hash = (HV *)SvRV(attribs);
+
+        attr = hv_fetch(attr_hash, "StrictlyTyped", (U32)13, 0);
+        if (attr && SvTRUE(*attr)) {
+            imp_sth->fbh[field-1].bind_flags |= DBIstcf_STRICT;
+        }
+
+        attr = hv_fetch(attr_hash, "DiscardString", (U32)13, 0);
+        if (attr && SvTRUE(*attr)) {
+            imp_sth->fbh[field-1].bind_flags |= DBIstcf_DISCARD_STRING;
+        }
+    }
+#endif  /* DBIXS_REVISION >= 13590 */
+    return 1;
+}
+
 int
 dbd_db_disconnect(SV *dbh, imp_dbh_t *imp_dbh)
 {
Index: dbdimp.h
===================================================================
--- dbdimp.h    (revision 13624)
+++ dbdimp.h    (working copy)
@@ -113,7 +113,7 @@
        int                             fetch_offset;
        int                             fetch_position;
        int                     prefetch_memory;        /* OCI_PREFETCH_MEMORY*/
-       int                             prefetch_rows;          /* OCI_PREFETCH_ROWS
+    int                                prefetch_rows;          /* OCI_PREFETCH_ROWS */

        /* array fetch: state variables */
        int                             row_cache_off;
        int                     rs_fetch_count;         /*fetch count*/
@@ -194,6 +194,9 @@
        int                     piece_lob;  /*use piecewise fetch for lobs*/
        /* Our storage space for the field data as it's fetched */
        sword           ftype;  /* external datatype we wish to get     */
+        IV            req_type;                /* type passed to bind_col */
+        UV            bind_flags;               /* flags passed to bind_col */
+    
        fb_ary_t        *fb_ary ;       /* field buffer array                   */
        /* if this is an embedded object we use this */
        fbh_obj_t       *obj;
@@ -376,6 +379,7 @@
 #define dbd_st_FETCH_attrib    ora_st_FETCH_attrib
 #define dbd_describe           ora_describe
 #define dbd_bind_ph                    ora_bind_ph
+#define dbd_st_bind_col         ora_st_bind_col
 #include "ocitrace.h"
 
 /* end */
Index: Oracle.h
===================================================================
--- Oracle.h    (revision 13624)
+++ Oracle.h    (working copy)
@@ -67,6 +67,7 @@
 int     dbd_db_do _((SV *sv, char *statement));
 int     dbd_db_commit     _((SV *dbh, imp_dbh_t *imp_dbh));
 int     dbd_db_rollback   _((SV *dbh, imp_dbh_t *imp_dbh));
+int dbd_st_bind_col(SV *sth, imp_sth_t *imp_sth, SV *col, SV *ref, IV type, SV *attribs);
 int     dbd_db_disconnect _((SV *dbh, imp_dbh_t *imp_dbh));
 void dbd_db_destroy    _((SV *dbh, imp_dbh_t *imp_dbh));
 int     dbd_db_STORE_attrib _((SV *dbh, imp_dbh_t *imp_dbh, SV *keysv, SV *valuesv));

Changes make it to DBD::Oracle 1.24 RC4

These changes have made it into DBD::Oracle 1.24 RC4 so it is likely they will be in the final 1.24 release. However, to take advantage of this change you need a new DBI and that has not be released yet so the only current choice is to build DBI from the latest checked in version from subversion.

Changes made it to 1.24a release

These changes made it to the 1.24a release of DBD::Oracle. However, to use them you will need to get and install the lastest DBI from subversion first then recompile DBD::Oracle.