Addressed review comments

source/pdo_sqlsrv/pdo_stmt.cpp

@@ -595,23 +595,22 @@ int pdo_sqlsrv_stmt_execute( _Inout_ pdo_stmt_t *stmt TSRMLS_DC )
             stmt->row_count = 0;
             driver_stmt->column_count = 0;
             driver_stmt->row_count = 0;
-            driver_stmt->columns_rows_obtained = true;
         }
         else {
-            if (driver_stmt->columns_rows_obtained == false)
-            {
+            if (driver_stmt->column_count == ACTIVE_NUM_COLS_INVALID) {
                 stmt->column_count = core::SQLNumResultCols( driver_stmt TSRMLS_CC );
+                driver_stmt->column_count = stmt->column_count;
+            }
+            else {
+                stmt->column_count = driver_stmt->column_count;
+            }
 
+            if (driver_stmt->row_count == ACTIVE_NUM_ROWS_INVALID) {
                 // return the row count regardless if there are any rows or not
                 stmt->row_count = core::SQLRowCount( driver_stmt TSRMLS_CC );
-                
-                driver_stmt->column_count = stmt->column_count;
                 driver_stmt->row_count = stmt->row_count;
-                driver_stmt->columns_rows_obtained = true;
             }
-            else
-            {
-                stmt->column_count = driver_stmt->column_count;
+            else {
                 stmt->row_count = driver_stmt->row_count;
             }
         }
@@ -707,11 +706,21 @@ int pdo_sqlsrv_stmt_fetch( _Inout_ pdo_stmt_t *stmt, _In_ enum pdo_fetch_orienta
         SQLSMALLINT odbc_fetch_ori = pdo_fetch_ori_to_odbc_fetch_ori( ori );
         bool data = core_sqlsrv_fetch( driver_stmt, odbc_fetch_ori, offset TSRMLS_CC );
 
-        // support for the PDO rowCount method.  Since rowCount doesn't call a method, PDO relies on us to fill the 
-        // pdo_stmt_t::row_count member
-        if( driver_stmt->past_fetch_end) {
+        // support for the PDO rowCount method.  Since rowCount doesn't call a
+        // method, PDO relies on us to fill the pdo_stmt_t::row_count member
+        // The if condition was changed from 
+        // `driver_stmt->past_fetch_end || driver_stmt->cursor_type != SQL_CURSOR_FORWARD_ONLY` 
+        // because it caused SQLRowCount to be called at each fetch if using a non-forward cursor
+        // which is unnecessary and a performance hit
+        if( driver_stmt->past_fetch_end || driver_stmt->cursor_type == SQL_CURSOR_DYNAMIC) {
 
+            if (driver_stmt->row_count == ACTIVE_NUM_ROWS_INVALID) {
                 stmt->row_count = core::SQLRowCount( driver_stmt TSRMLS_CC );
+                driver_stmt->row_count = stmt->row_count;
+            }
+            else {
+                stmt->row_count = driver_stmt->row_count;
+            }
 
             // a row_count of -1 means no rows, but we change it to 0
             if( stmt->row_count == -1 ) {
@@ -1164,7 +1173,6 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC )
         
         driver_stmt->column_count = stmt->column_count;
         driver_stmt->row_count = stmt->row_count;
-        driver_stmt->columns_rows_obtained = true;
     }
     catch( core::CoreException& ) {
 

source/shared/core_sqlsrv.h

@@ -182,6 +182,11 @@ const int SQL_MAX_ERROR_MESSAGE_LENGTH = SQL_MAX_MESSAGE_LENGTH * 2;
 // max size of a date time string when converting from a DateTime object to a string
 const int MAX_DATETIME_STRING_LEN = 256;
 
+// identifier for whether or not we have obtained the number of rows and columns
+// of a result
+const short ACTIVE_NUM_COLS_INVALID = -99;
+const long ACTIVE_NUM_ROWS_INVALID = -99;
+
 // precision and scale for the date time types between servers
 const int SQL_SERVER_2005_DEFAULT_DATETIME_PRECISION = 23;
 const int SQL_SERVER_2005_DEFAULT_DATETIME_SCALE = 3;
@@ -1439,7 +1444,6 @@ struct sqlsrv_stmt : public sqlsrv_context {
     bool fetch_called;                    // Used by core_sqlsrv_get_field to return an informative error if fetch not yet called 
     int last_field_index;                 // last field retrieved by core_sqlsrv_get_field
     bool past_next_result_end;            // core_sqlsrv_next_result sets this to true when the statement goes beyond the last results
-    bool columns_rows_obtained;           // Whether or not SQLNumResultCols and SQLRowCount have been called for the active result
     short column_count;                   // Number of columns in the current result set obtained from SQLNumResultCols
     long row_count;                       // Number of rows in the current result set obtained from SQLRowCount
     unsigned long query_timeout;          // maximum allowed statement execution time

source/shared/core_stmt.cpp

@@ -140,9 +140,8 @@ sqlsrv_stmt::sqlsrv_stmt( _In_ sqlsrv_conn* c, _In_ SQLHANDLE handle, _In_ error
     fetch_called( false ),
     last_field_index( -1 ),
     past_next_result_end( false ),
-    columns_rows_obtained( false ),
-    column_count( 0 ),
-    row_count( 0 ),
+    column_count( ACTIVE_NUM_COLS_INVALID ),
+    row_count( ACTIVE_NUM_ROWS_INVALID ),
     query_timeout( QUERY_TIMEOUT_INVALID ),
     date_as_string(false),
     format_decimals(false),       // no formatting needed
@@ -228,9 +227,8 @@ void sqlsrv_stmt::new_result_set( TSRMLS_D )
     this->past_next_result_end = false;
     this->past_fetch_end = false;
     this->last_field_index = -1;
-    this->columns_rows_obtained = false;
-    this->column_count = 0;
-    this->row_count = 0;
+    this->column_count = ACTIVE_NUM_COLS_INVALID;
+    this->row_count = ACTIVE_NUM_ROWS_INVALID;
 
     // delete any current results
     if( current_results ) {
@@ -828,7 +826,7 @@ bool core_sqlsrv_fetch( _Inout_ sqlsrv_stmt* stmt, _In_ SQLSMALLINT fetch_orient
         // First time only
         if ( !stmt->fetch_called ) {
             SQLSMALLINT has_fields;
-            if (stmt->columns_rows_obtained) {
+            if (stmt->column_count != ACTIVE_NUM_COLS_INVALID) {
                 has_fields = stmt->column_count;
             } else {
                 has_fields = core::SQLNumResultCols( stmt TSRMLS_CC );
@@ -1078,21 +1076,25 @@ void core_sqlsrv_get_field( _Inout_ sqlsrv_stmt* stmt, _In_ SQLUSMALLINT field_i
 
 bool core_sqlsrv_has_any_result( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC )
 {
-    SQLSMALLINT num_cols, rows_affected;
-    if (stmt->columns_rows_obtained)
-    {
+    SQLSMALLINT num_cols;
+    SQLLEN rows_affected;
+    
+    if (stmt->column_count != ACTIVE_NUM_COLS_INVALID) {
         num_cols = stmt->column_count;
-        rows_affected = stmt->row_count;
     }
-    else
-    {
+    else {
         // Use SQLNumResultCols to determine if we have rows or not
         num_cols = core::SQLNumResultCols( stmt TSRMLS_CC );
         stmt->column_count = num_cols;
+    }
+    
+    if (stmt->row_count != ACTIVE_NUM_ROWS_INVALID) {
+        rows_affected = stmt->row_count;
+    }
+    else {
         // Use SQLRowCount to determine if there is a rows status waiting
         rows_affected = core::SQLRowCount( stmt TSRMLS_CC );
         stmt->row_count = rows_affected;
-        stmt->columns_rows_obtained = true;
     }
     
     return (num_cols != 0) || (rows_affected > 0);

source/sqlsrv/stmt.cpp

@@ -1791,7 +1791,7 @@ SQLSMALLINT get_resultset_meta_data(_Inout_ sqlsrv_stmt * stmt)
 
     if (num_cols == 0) {
         getMetaData = true;
-        if (stmt->columns_rows_obtained == false) {
+        if (stmt->column_count == ACTIVE_NUM_COLS_INVALID) {
             num_cols = core::SQLNumResultCols(stmt TSRMLS_CC);
         } else {
             num_cols = stmt->column_count;

source/sqlsrv/util.cpp

@@ -196,7 +196,7 @@ ss_error SS_ERRORS[] = {
           
     {
         SQLSRV_ERROR_NO_FIELDS,
-        { IMSSP, (SQLCHAR*)"The active result for the query contains no fields, so no result set was created.", -28, false }
+        { IMSSP, (SQLCHAR*)"The active result for the query contains no fields.", -28, false }
     },
 
     {

test/functional/sqlsrv/sqlsrv_empty_result_error.phpt

@@ -12,7 +12,7 @@ require_once("MsCommon.inc");
 // These are the error messages we expect at various points below
 $errorNoMoreResults = "There are no more results returned by the query.";
 $errorNoMoreRows    = "There are no more rows in the active result set.  Since this result set is not scrollable, no more data may be retrieved.";
-$errorNoFields      = "The active result for the query contains no fields, so no result set was created.";
+$errorNoFields      = "The active result for the query contains no fields.";
 
 // Variable function gets an error message that depends on the OS
 function getFuncSeqError()

test/functional/sqlsrv/test_warning_errors3.phpt

@@ -128,8 +128,8 @@ Array
             [SQLSTATE] => IMSSP
             [1] => -28
             [code] => -28
-            [2] => The active result for the query contains no fields, so no result set was created.
-            [message] => The active result for the query contains no fields, so no result set was created.
+            [2] => The active result for the query contains no fields.
+            [message] => The active result for the query contains no fields.
         )
 
 )