Work on REF CURSOR crashes and errors

Author: cjbj

doc/api.md

@@ -2892,7 +2892,8 @@ Oracle REF CURSORS can be fetched in node-oracledb by binding a
 [`getRow()`](#getrow) or [`getRows()`](getrows).  When all rows have
 been fetched, or the application does not want to continue getting
 more rows, then the result set must be freed using
-[`close()`](#close).
+[`close()`](#close).  If the cursor is not initialized by the procedure, the
+[`ResultSet`](#resultsetclass) object still requires to be closed.
 
 When using Oracle Database 11gR2 or greater, then
 [`prefetchRows`](#propdbprefetchrows) can be used to tune the

src/dpi/include/dpiStmt.h

@@ -99,6 +99,20 @@ typedef enum
   DpiIntervalArray              /* external only */
 } DpiDataType;
 
+
+/* OCI Stmt Handle state
+ *  For REFCURSORS state should be DpiStmtStateExecuted
+ */
+typedef enum
+{
+  DpiStmtStateUndefined   = -1,     /* Undefined */
+  DpiStmtStateInitialized = 0x001,  /* Initialized */
+  DpiStmtStateExecuted,             /* Executed */
+  DpiStmtStateEndOfFetch            /* End-Of-Fetch */
+} DpiStmtState;
+
+
+
 /*
  * For 11g/12c Compatability BIND/DEFINE calls expect ub8 in 12c & ub4 in 11g
  * Using this type makes is compile-time selction of 11g or 12c.
@@ -187,6 +201,7 @@ class Stmt
 
   virtual OCIError *getError () = 0;
 
+  virtual DpiStmtState getState () = 0;
 
   virtual ~Stmt(){};
 

src/dpi/src/dpiStmtImpl.cpp

@@ -88,7 +88,8 @@ StmtImpl::StmtImpl (EnvImpl *env, OCIEnv *envh, ConnImpl *conn,
 
   try : conn_(conn), errh_(NULL), svch_(svch),
         stmth_(NULL), numCols_ (0),meta_(NULL), stmtType_ (DpiStmtUnknown),
-        isReturning_(false), isReturningSet_(false), refCursor_(false)
+        isReturning_(false), isReturningSet_(false), refCursor_(false),
+        state_(0)
 {
   // create an OCIError object for this execution
   ociCallEnv (OCIHandleAlloc ((void *)envh, (dvoid **)&errh_,
@@ -716,5 +717,40 @@ bool StmtImpl::isReturning ()
   return isReturning_;
 }
 
+/*****************************************************************************/
+/*
+   DESCRIPTION
+     To obtain the OCI statement handle state
+
+   PARAMETERS
+     -None-
+
+   RETURNS
+     One of the possible values DpiStmtState
+       (DpiStmtStateInitialized, DpiStmtStateExecute, DpiStmtEndOfFetch)
+*/
+DpiStmtState StmtImpl::getState ()
+{
+  DpiStmtState ret = DpiStmtStateUndefined ;
+
+  if ( !state_ )
+  {
+    ociCall (OCIAttrGet (stmth_, OCI_HTYPE_STMT, (ub4*)&state_, NULL,
+                         OCI_ATTR_STMT_STATE, errh_ ), errh_ );
+  }
+
+  if ( ( state_ & OCI_STMT_STATE_INITIALIZED ) == OCI_STMT_STATE_INITIALIZED )
+    ret = DpiStmtStateInitialized;
+
+  if ( ( state_ & OCI_STMT_STATE_EXECUTED ) == OCI_STMT_STATE_EXECUTED )
+    ret = DpiStmtStateExecuted;
+
+  if ( ( state_ & OCI_STMT_STATE_END_OF_FETCH ) ==
+       OCI_STMT_STATE_END_OF_FETCH )
+    ret = DpiStmtStateEndOfFetch;
+
+  return ret;
+}
+
 
 /* end of file dpiStmtImpl.cpp */

src/dpi/src/dpiStmtImpl.h

@@ -94,6 +94,9 @@ class StmtImpl : public Stmt
 
   virtual OCIError *     getError () { return errh_;  }
 
+  virtual DpiStmtState   getState ();
+
+
   // Is the SQL statement DML or not ?
   virtual inline bool isDML () const
   {
@@ -134,6 +137,7 @@ class StmtImpl : public Stmt
   bool           isReturning_;     // Does the stmt has RETURNING INTO clause?
   bool           isReturningSet_;  // Has isReturning_ flag queried & set.
   bool           refCursor_;       // refCursor or not.
+  unsigned long  state_;           // OCI Stmt State
 };
 
 

src/njs/src/njsConnection.cpp

@@ -593,6 +593,16 @@ void Connection::GetBindUnit (Handle<Value> val, Bind* bind,
       goto exitGetBindUnit;
     }
 
+    /* REFCURSOR(s) are supported only as OUT Binds now */
+    if ( bind->type == DATA_CURSOR && dir != BIND_OUT )
+    {
+      executeBaton->error = NJSMessages::getErrorMsg (
+                                            errInvalidPropertyValueInParam,
+                                            "type", 2 ) ;
+      goto exitGetBindUnit;
+    }
+
+
     Local<Value> element = bind_unit->Get(NanNew<v8::String>("val"));
     switch(dir)
     {
@@ -919,6 +929,20 @@ void Connection::Async_Execute (uv_work_t *req)
          bind->dttmarr = NULL;
        }
      }
+
+     /* For each OUT Binds of CURSOR type, get the number of columns */
+     for ( unsigned int b = 0; b < executeBaton->binds.size (); b ++ )
+     {
+       Bind *bind = executeBaton->binds[b];
+
+       if ( bind->isOut && bind->type == dpi::DpiRSet )
+       {
+         bind->flags =
+           (((Stmt*)bind->value)->getState () == DpiStmtStateExecuted ) ?
+               BIND_FLAGS_STMT_READY : BIND_FLAGS_STMT_NOT_READY;
+       }
+     }
+
      // process any lob descriptor out binds
      Connection::Descr2protoILob ( executeBaton, 0, 0);
     }
@@ -969,6 +993,14 @@ void Connection::PrepareAndBind (eBaton* executeBaton)
         for(unsigned int index = 0 ;index < executeBaton->binds.size();
             index++)
         {
+          if ( executeBaton->binds[index]->isOut &&
+               executeBaton->stmtIsReturning &&
+               executeBaton->binds[index]->type == dpi::DpiRSet )
+          {
+            executeBaton->error = NJSMessages::getErrorMsg (
+                                                       errInvalidResultSet ) ;
+          }
+
           // Allocate for OUT Binds
           // For DML Returning, allocation happens through callback.
           if ( executeBaton->binds[index]->isOut &&
@@ -1775,9 +1807,12 @@ void Connection::Async_AfterExecute(uv_work_t *req)
           result->Set(NanNew<v8::String>("rows"), NanUndefined());
           Handle<Object> resultSet = NanNew(ResultSet::resultSetTemplate_s)->
                                 GetFunction() ->NewInstance();
+
+          /* ResultSet case, the statement object is ready for fetching */
          (ObjectWrap::Unwrap<ResultSet> (resultSet))->
                                   setResultSet( executeBaton->dpistmt,
-                                                executeBaton );
+                                                executeBaton,
+                                                BIND_FLAGS_STMT_READY );
           result->Set(NanNew<v8::String>("resultSet"), resultSet );
         }
         else
@@ -2010,7 +2045,8 @@ Handle<Value> Connection::GetValueRefCursor ( eBaton *executeBaton,
                             GetFunction() ->NewInstance();
     (ObjectWrap::Unwrap<ResultSet> (resultSet))->
                        setResultSet( (dpi::Stmt*)(bind->value),
-                                     executeBaton );
+                                     executeBaton,
+                                     bind->flags );
 
     // set the prefetch on the cursor object
     ((dpi::Stmt*)(bind->value))->prefetchRows(executeBaton->prefetchRows);

src/njs/src/njsConnection.h

@@ -66,6 +66,9 @@ using namespace dpi;
 class Connection;
 class ProtoILob;
 
+/* Flags Constants */
+#define BIND_FLAGS_STMT_NOT_READY   0    /* REFCURSOR stmt not-fetch-able */
+#define BIND_FLAGS_STMT_READY       1    /* REFCURSOR stmt fetch-able */
 
 /**
 * Structure used for binds
@@ -84,11 +87,12 @@ typedef struct Bind
   bool                isInOut;          // Date/Timestamp needs this info
   unsigned int        rowsReturned;     /* number rows returned for
                                            the bind (DML RETURNING) */
+  unsigned long       flags;            // Flags applicable for this BIND
   dpi::DateTimeArray* dttmarr;
 
   Bind () : key(""), value(NULL), extvalue (NULL), len(NULL), len2(NULL),
             maxSize(0), type(0), ind(NULL), isOut(false), isInOut(false),
-            rowsReturned(0), dttmarr ( NULL )
+            rowsReturned(0), flags(BIND_FLAGS_STMT_NOT_READY), dttmarr ( NULL )
   {}
 }Bind;
 

src/njs/src/njsResultSet.cpp

@@ -68,13 +68,27 @@ Persistent<FunctionTemplate> ResultSet::resultSetTemplate_s;
      stmt         -  dpi statement
      executeBaton - eBaton structure
 */
-void ResultSet::setResultSet ( dpi::Stmt *stmt, eBaton *executeBaton )
+void ResultSet::setResultSet ( dpi::Stmt *stmt, eBaton *executeBaton,
+                               unsigned long flags )
 {
   this->dpistmt_       = stmt;
   this->dpienv_        = executeBaton->dpienv;
   this->njsconn_       = executeBaton->njsconn;
-  this->meta_          = stmt->getMetaData();
-  this->numCols_       = this->dpistmt_->numCols();
+  if ( ( flags & BIND_FLAGS_STMT_READY ) == BIND_FLAGS_STMT_READY )
+  {
+    this->meta_        = stmt->getMetaData();
+    this->numCols_     = this->dpistmt_->numCols();
+  }
+  else if ( ( flags & BIND_FLAGS_STMT_READY ) == BIND_FLAGS_STMT_NOT_READY )
+  {
+    /* 
+     * This could happen in REFCURSOR case, when the stored procedure
+     * did not return a valid handle
+     */
+    this->numCols_     = 0;
+    this->meta_        = NULL;
+  }
+
   this->state_         = INACTIVE;
   this->outFormat_     = executeBaton->outFormat;
   this->fetchRowCount_ = 0;
@@ -173,6 +187,11 @@ NAN_PROPERTY_GETTER(ResultSet::GetMetaData)
   ResultSet* njsResultSet  = ObjectWrap::Unwrap<ResultSet>(args.Holder());
   string msg;
 
+  if ( njsResultSet->numCols_ == 0 )
+  {
+    njsResultSet->state_ = INVALID;
+  }
+
   if(!njsResultSet->njsconn_->isValid())
   {
     msg = NJSMessages::getErrorMsg ( errInvalidConnection );
@@ -234,6 +253,11 @@ NAN_METHOD(ResultSet::GetRow)
   getRowsBaton->njsRS     = njsResultSet;
   NanAssignPersistent(getRowsBaton->cb, callback );
 
+  if ( njsResultSet->numCols_ == 0 )
+  {
+    njsResultSet->state_ = INVALID ;
+  }
+
   if(njsResultSet->state_ == INVALID)
   {
     getRowsBaton->error = NJSMessages::getErrorMsg ( errInvalidResultSet );
@@ -279,6 +303,11 @@ NAN_METHOD(ResultSet::GetRows)
   getRowsBaton->njsRS   = njsResultSet;
   NanAssignPersistent(getRowsBaton->cb, callback );
 
+  if ( njsResultSet->numCols_ == 0 )
+  {
+    njsResultSet->state_ = INVALID;
+  }
+
   if(njsResultSet->state_ == INVALID)
   {
     getRowsBaton->error = NJSMessages::getErrorMsg ( errInvalidResultSet );

src/njs/src/njsResultSet.h

@@ -104,7 +104,8 @@ class ResultSet: public ObjectWrap {
 
    static void Init(Handle<Object> target);
 
-   void setResultSet ( dpi::Stmt *dpistmt, eBaton *executebaton );
+   void setResultSet ( dpi::Stmt *dpistmt, eBaton *executebaton,
+                       unsigned long flags );
 
 
    // Define ResultSet Constructor
@@ -135,21 +136,23 @@ class ResultSet: public ObjectWrap {
   static void clearFetchBuffer( Define* defineBuffers,
                                 unsigned int numCols );
 
-
-   dpi::Stmt            *dpistmt_;
-   dpi::Env             *dpienv_;
-   Connection           *njsconn_;
-   State                state_;
-   bool                 rsEmpty_;
-   Define               *defineBuffers_;
-   unsigned int         numCols_;
-   unsigned int         fetchRowCount_;
-   unsigned int         outFormat_;
-   const dpi::MetaData  *meta_;
-   DataType             *fetchAsStringTypes_;
-   unsigned int         fetchAsStringTypesCount_;
-   FetchInfo            *fetchInfo_;
-   unsigned int         fetchInfoCount_;
+  static void Init ();
+  
+
+  dpi::Stmt            *dpistmt_;
+  dpi::Env             *dpienv_;
+  Connection           *njsconn_;
+  State                state_;
+  bool                 rsEmpty_;
+  Define               *defineBuffers_;
+  unsigned int         numCols_;
+  unsigned int         fetchRowCount_;
+  unsigned int         outFormat_;
+  const dpi::MetaData  *meta_;
+  DataType             *fetchAsStringTypes_;
+  unsigned int         fetchAsStringTypesCount_;
+  FetchInfo            *fetchInfo_;
+  unsigned int         fetchInfoCount_;
 };