Re: Problem with procedures returning a SYS_REFCURSOR which is not open/executed - possible fixes

2013-01-12 Thread Martin J. Evans

On 11/01/2013 21:22, John Scoles wrote:

Hmm you sure pick the head scratchers.
  
My first thoughts where why whould you make such a procedure?? But then I relaisze it could have some use when prototying/develpopeing ect.
  

No, this is production.

Sometimes there is a result-set to return and sometimes there is not. 
Our situation here is perhaps a little unusual in that there is no read 
or write access to any table in the database for anyone other than the 
owner of the package - definer rights. As a result there is a procedure 
to return any result-set the outside code needs. In this case the 
outside knows some work needs to be done but not what it is and usually 
does this:


alerted some work needs to be done
calls proc to get the work
depending on the work it might call another proc
once work is done calls a proc to say it is complete and audit it

I'm skipping the last 2 steps for some work by doing the dispatching in 
the database thus cutting out to 2 trips to the database. The downside 
is some of the work returns a cursor and some does not.



Looking at
  
'DBD::Oracle attempts to describe statements which are initialized but not executed'
  
We might be able to do something here.  I remember working on a patch some time ago (ie 2008) to get the describe after the execute then do all the binding at the end of an execute etc as the newer versions of Oracle return this data for you.  Not sure it that is the right time for this on as if I remember correctly there was alot of reprogrmming involved.
  
Doing the describe before the execute is a pain and the current describe 
code makes all the decisions on how columns will be bound which means a 
type to bind_col is ignored. Hence rt's like:


https://rt.cpan.org/Ticket/Display.html?id=71810
ora_type = ORA_RAW doesn't work for select

where the reporter has a raw column which is bound as a string so Oracle 
converts it to hex and the result is twice as long. Of course we could 
just change the default binding of raw to bytes but I was reticent to do 
that since the code had been that way a long time and people might be 
relying on it. If the describe was afterwards the reporter could just 
specify raw in the bind_col call thus not upsetting anyone else.


I got away with implementing strictly typed etc on bind_col because they 
are applied afterwards.



Will have to look into that one.
  
Anyway I agree that pp_exec_rset is the place to make your change  as the is safe area to do it.  Perhaps we can delay the pre_exec part untill after the inital query is executed by that time you will know if you have a ref that you will need to bind and return??
That was one of my thoughts also. In actual fact, I'd rather have undef 
back for the cursor when there is no open cursor returned, it seems more 
intuitive to me and avoids creating a new sth for no purpose. My only 
concern was if it was at all possible to return an initialised cursor 
for a valid result-set which was just not executed yet - not that I know 
if you can do this or how. I will hopefully try and find the time to 
look at that.
  
  
Been a while since I looked at this part of the code so bear with me if I sound a little rusty.
  
Cheere

John
Anyway, the change I arrived at does not seem to have any negative 
affects on the test suite or our application so far. If I can find a 
better way of doing it which is not a total overhaul of dbd_describe I 
will but as I seem to be the only one with this issue I did not want to 
risk breaking other things.


Martin
  
  
  

  


Date: Fri, 11 Jan 2013 16:04:13 +
From: martin.ev...@easysoft.com
To: dbi-dev@perl.org
Subject: Problem with procedures returning a SYS_REFCURSOR which is not 
open/executed - possible fixes

I am using DBD::Oracle and calling a procedure which returns a reference 
cursor. However, sometimes the reference cursor is not opened and only the 
procedure knows this. The problem is if I call the procedure from DBD::Oracle 
and the cursor is not opened I get an Oracle error saying the cursor is not 
executed:

test procedure:
procedure p_n2(pcur OUT SYS_REFCURSOR) AS
begin
pcur := NULL;
end;

example perl:
my $s = $h-prepare(q/begin mypkg.p_n2(?); end;/);
$s-bind_param_inout(1, \my $cursor, 100, {ora_type = ORA_RSET});
$s-execute; # errors

The error occurs because DBD::Oracle attempts to call dbd_describe on the 
returned cursor (before perl land even sees it) and that code does things like 
call OCIAttrGet for PARAM_COUNT etc which Oracle disallows if the statement is 
not executed.

An easy solution is to just open an empty cursor if the procedure cannot open a 
real one by doing something like:

open pcur for select 1 from dual;

but I don't like that as DBD::Oracle will make dozens of calls and do quite a 
bit of work in dbd_describe which is wasting time and the purpose of the change 
to my procedure is to speed this application up not slow it down.

Just to be clear in case anyone thinks I've just invented a 

Re: Problem with procedures returning a SYS_REFCURSOR which is not open/executed - possible fixes

2013-01-12 Thread Martin J. Evans

On 11/01/2013 19:28, Tim Bunce wrote:

On Fri, Jan 11, 2013 at 04:04:13PM +, 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