Re: Problem with procedures returning a SYS_REFCURSOR which is not open/executed - possible fixes
"Martin J. Evans" <[email protected]>
| Newsgroups | gmane.comp.lang.perl.modules.dbi.sybase.devel |
|---|---|
| Message-ID | <[email protected]> |
On 11/01/2013 19:28, Tim Bunce wrote:
> On Fri, Jan 11, 2013 at 04:04:13PM +0000, Martin J. Evans wrote:
>> My second sub attempt was to outright lie and set dbd_describe_done
>> and leave Active off so from perl land I just need to test Active
>> flag. This works and is a safer change since it ONLY applies to sth
>> handles magicked into existence for returned cursors. Also, if you
>> attempt to do anything else with the sth it errors as it should:
>>
>> DBD::Oracle::st fetch failed: ERROR no statement executing (perhaps
>> you need to call execute first) at bz1245.pl line 16.
>>
>> Wondered if anyone else had any thoughts on this.
> Sounds good to me. Thanks for looking after this Martin.
>
> Tim.
I've now got 2 ways to fix this issue. The first way is described above
and is a relatively small change. When pp_exec_rset is called post
execute it simply looks at the Oracle statement state and if it is only
initialised and not executed it leaves Active off and sets done_desc to
stop DBD::Oracle attempting to call dbd_describe. On the outside all
your Perl needs to do is test Active before attempting to use the cursor.
Advantages: small change unlikely to have any repercussions since we
still return a sth and if you attempt to use a non-executed sth it will
error with not executed. Fixes the problem I'm trying to fix.
Disadvantages: still creates a sth which is useless
Index: dbdimp.c
===================================================================
--- dbdimp.c (revision 15554)
+++ dbdimp.c (working copy)
@@ -2737,10 +2737,11 @@
DBIc_LOGPIO(imp_sth),
" pp_exec_rset bind %s - allocated %s...\n",
phs->name, neatsvpv(phs->sv, 0));
-
}
else { /* post-execute - setup the statement handle */
dTHR;
+ ub4 stmt_state = 99;
+ sword status;
SV * sth_csr = phs->sv;
D_impdata(imp_sth_csr, imp_sth_t, sth_csr);
@@ -2771,7 +2772,23 @@
imp_sth_csr->stmt_type = OCI_STMT_SELECT;
DBIc_IMPSET_on(imp_sth_csr);
- /* set ACTIVE so dbd_describe doesn't do explicit OCI describe */
+ OCIAttrGet_stmhp_stat(imp_sth_csr, &stmt_state, 0,
OCI_ATTR_STMT_STATE,
+ if (status != OCI_SUCCESS) {
+ oci_error(sth, imp_sth->errhp, status, "OCIAttrGet
OCI_ATTR_STMT_ST
+ return 0;
+ }
+ if (DBIc_DBISTATE(imp_sth)->debug >= 3 || dbd_verbose >= 3 ) {
+ /* initialized=1, executed=2, end of fetch=3 */
+ PerlIO_printf(
+ DBIc_LOGPIO(imp_sth),
+ " statement state: %u\n", stmt_state);
+ }
+ if (stmt_state == OCI_STMT_STATE_INITIALIZED) {
+ imp_sth_csr->done_desc = 1;
+ return 1;
+ }
+
+ /* set ACTIVE so dbd_describe doesn't do explicit OCI describe */
DBIc_ACTIVE_on(imp_sth_csr);
if (!dbd_describe(sth_csr, imp_sth_csr)) {
return 0;
Second solution is a bit more involved but I think better since a
non-executed sth is not returned - instead undef is returned.
Advantages: fixes problem and does not create a useless sth
Disadvantages: touches the code which gets run if the returned cursor is
executed although I've mainly just moved it to the post execute path.
Index: dbdimp.c
===================================================================
--- dbdimp.c (revision 15554)
+++ dbdimp.c (working copy)
@@ -2666,10 +2666,6 @@
dTHX;
if (pre_exec) { /* pre-execute - allocate a statement handle */
- dSP;
- D_imp_dbh_from_sth;
- HV *init_attr = newHV();
- int count;
sword status;
if (DBIc_DBISTATE(imp_sth)->debug >= 3 || dbd_verbose
>= 3 )
@@ -2691,7 +2687,6 @@
OCIHandleAlloc_ok(imp_sth, imp_sth->envhp,
&phs->desc_h, phs->desc_t, status);
}
-
phs->progv = (char*)&phs->desc_h;
phs->maxlen = 0;
@@ -2714,6 +2709,38 @@
return 0;
}
+ }
+ else { /* post-execute - setup the statement handle */
+ dTHR;
+ dSP;
+ D_imp_dbh_from_sth;
+ HV *init_attr = newHV();
+ int count;
+ ub4 stmt_state = 99;
+ sword status;
+ SV * sth_csr;
+
+ /* Before we go to the bother of attempting to allocate a new sth
+ for this cursor make sure the Oracle sth is executed i.e.,
+ the returned cursor may never have been opened */
+ OCIAttrGet_stmhp_stat2(imp_sth, (OCIStmt*)phs->desc_h,
&stmt_state, 0,
+ OCI_ATTR_STMT_STATE, status);
+ if (status != OCI_SUCCESS) {
+ oci_error(sth, imp_sth->errhp, status, "OCIAttrGet
OCI_ATTR_STMT_STATE");
+ return 0;
+ }
+ if (DBIc_DBISTATE(imp_sth)->debug >= 3 || dbd_verbose >= 3 ) {
+ /* initialized=1, executed=2, end of fetch=3 */
+ PerlIO_printf(
+ DBIc_LOGPIO(imp_sth),
+ " returned cursor/statement state: %u\n", stmt_state);
+ }
+ if (stmt_state == OCI_STMT_STATE_INITIALIZED) {
+ phs->sv = newSV(0); /* undef */
+ return 1;
+ }
+
+ /* Now we know we have an executed cursor create a new sth */
ENTER;
SAVETMPS;
PUSHMARK(SP);
@@ -2738,11 +2765,7 @@
" pp_exec_rset bind %s - allocated %s...\n",
phs->name, neatsvpv(phs->sv, 0));
- }
- else { /* post-execute - setup the statement handle */
- dTHR;
- SV * sth_csr = phs->sv;
- D_impdata(imp_sth_csr, imp_sth_t, sth_csr);
+ sth_csr = phs->sv;
if (DBIc_DBISTATE(imp_sth)->debug >= 3 || dbd_verbose
>= 3 )
PerlIO_printf(
@@ -2750,32 +2773,36 @@
" bind %s - initialising new %s for cursor
0x%lx...\n",
phs->name, neatsvpv(sth_csr,0), (unsigned
long)phs->progv);
- /* copy appropriate handles and atributes from parent
statement */
- imp_sth_csr->envhp = imp_sth->envhp;
- imp_sth_csr->errhp = imp_sth->errhp;
- imp_sth_csr->srvhp = imp_sth->srvhp;
- imp_sth_csr->svchp = imp_sth->svchp;
- imp_sth_csr->auto_lob = imp_sth->auto_lob;
- imp_sth_csr->pers_lob = imp_sth->pers_lob;
- imp_sth_csr->clbk_lob = imp_sth->clbk_lob;
- imp_sth_csr->piece_size = imp_sth->piece_size;
- imp_sth_csr->piece_lob = imp_sth->piece_lob;
- imp_sth_csr->is_child = 1; /*no prefetching on a
cursor or sp*/
+ {
+ D_impdata(imp_sth_csr, imp_sth_t, sth_csr); /* TO_DO */
+ /* copy appropriate handles and atributes from parent
statement */
+ imp_sth_csr->envhp = imp_sth->envhp;
+ imp_sth_csr->errhp = imp_sth->errhp;
+ imp_sth_csr->srvhp = imp_sth->srvhp;
+ imp_sth_csr->svchp = imp_sth->svchp;
+ imp_sth_csr->auto_lob = imp_sth->auto_lob;
+ imp_sth_csr->pers_lob = imp_sth->pers_lob;
+ imp_sth_csr->clbk_lob = imp_sth->clbk_lob;
+ imp_sth_csr->piece_size = imp_sth->piece_size;
+ imp_sth_csr->piece_lob = imp_sth->piece_lob;
+ imp_sth_csr->is_child = 1; /*no prefetching on a
cursor or sp*/
- /* assign statement handle from placeholder descriptor */
- imp_sth_csr->stmhp = (OCIStmt*)phs->desc_h;
- phs->desc_h = NULL; /* tell phs that we
own it now */
- /* force stmt_type since OCIAttrGet(OCI_ATTR_STMT_TYPE) doesn't
work! */
- imp_sth_csr->stmt_type = OCI_STMT_SELECT;
- DBIc_IMPSET_on(imp_sth_csr);
+ /* assign statement handle from placeholder descriptor */
+ imp_sth_csr->stmhp = (OCIStmt*)phs->desc_h;
+ phs->desc_h = NULL; /* tell phs that we
own it now */
- /* set ACTIVE so dbd_describe doesn't do explicit OCI describe */
- DBIc_ACTIVE_on(imp_sth_csr);
- if (!dbd_describe(sth_csr, imp_sth_csr)) {
- return 0;
- }
+ /* force stmt_type since OCIAttrGet(OCI_ATTR_STMT_TYPE)
doesn't work! */
+ imp_sth_csr->stmt_type = OCI_STMT_SELECT;
+ DBIc_IMPSET_on(imp_sth_csr);
+
+ /* set ACTIVE so dbd_describe doesn't do explicit OCI
describe */
+ DBIc_ACTIVE_on(imp_sth_csr);
+ if (!dbd_describe(sth_csr, imp_sth_csr)) {
+ return 0;
+ }
+ }
}
return 1;
Index: ocitrace.h
===================================================================
--- ocitrace.h (revision 15554)
+++ ocitrace.h (working copy)
@@ -293,9 +293,9 @@
#define OCIAttrGet_log_stat(impxxh, th,ht,ah,sp,at,eh,stat) \
stat = OCIAttrGet(th,ht,ah,sp,at,eh); \
(DBD_OCI_TRACEON(impxxh)) ?
PerlIO_printf(DBD_OCI_TRACEFP(impxxh), \
- "%sAttrGet(%p,%s,%p,%p,%s,%p)=%s\n", \
+ "%sAttrGet(%p,%s,%p,%p,%s,%p)=%s (%s,
%d)\n", \
OciTp,
(void*)th,oci_hdtype_name(ht),(void*)ah,pul_t(sp),oci_attr_name(at),(void*)eh,\
- oci_status_name(stat)),stat : stat
+ oci_status_name(stat), __FILE__, __LINE__),stat : stat
#define OCIAttrGet_d_log_stat(impsth, th,ht,ah,sp,at,eh,stat) \
stat = OCIAttrGet(th,ht,ah,sp,at,eh); \
@@ -317,6 +317,10 @@
OCIAttrGet_log_stat(imp_sth, imp_sth->stmhp,
OCI_HTYPE_STMT, \
(void*)(p1), (l), (a), imp_sth->errhp, stat)
+#define OCIAttrGet_stmhp_stat2(imp_sth, stmhp, p1, l, a,
stat) \
+ OCIAttrGet_log_stat(imp_sth, stmhp, OCI_HTYPE_STMT, \
+ (void*)(p1), (l), (a), imp_sth->errhp, stat)
+
#define OCIAttrSet_log_stat(impxxh,th,ht,ah,s1,a,eh,stat) \
stat=OCIAttrSet(th,ht,ah,s1,a,eh); \
(DBD_OCI_TRACEON(impxxh)) ?
PerlIO_printf(DBD_OCI_TRACEFP(impxxh), \
martin@bragi:~/svn/dbd-oracle/trunk$
Both pass all of the test suite and currently work with our application
which binds a lot of output cursors.
Obviously I favour the second one as less work is done when no executed
cursor is returned. However, I'd be happier if other people using output
cursors tried it.
All patches against subversion trunk.
Martin