From a8a0146671f9e0b1609911c58fa4473f6d4a8425 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Tue, 2 Apr 2019 16:08:53 -0700 Subject: [PATCH 1/8] Removed forward cursor condition --- source/pdo_sqlsrv/pdo_stmt.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 773ad953..33932d7e 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -694,7 +694,7 @@ int pdo_sqlsrv_stmt_fetch( _Inout_ pdo_stmt_t *stmt, _In_ enum pdo_fetch_orienta // 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 || driver_stmt->cursor_type != SQL_CURSOR_FORWARD_ONLY ) { + if( driver_stmt->past_fetch_end) {// || driver_stmt->cursor_type != SQL_CURSOR_FORWARD_ONLY ) { stmt->row_count = core::SQLRowCount( driver_stmt TSRMLS_CC ); From 4b6b650db11f4589304d1ed4fb28eb8ab4ff7122 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 5 Apr 2019 12:34:33 -0700 Subject: [PATCH 2/8] Added row and column count checks --- source/pdo_sqlsrv/pdo_stmt.cpp | 19 ++++++++++++++++++ source/shared/core_sqlsrv.h | 6 ++++-- source/shared/core_stmt.cpp | 35 +++++++++++++++++++++++++++++----- source/sqlsrv/stmt.cpp | 4 ++++ 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 33932d7e..906f73fd 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -593,12 +593,27 @@ int pdo_sqlsrv_stmt_execute( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) if ( execReturn == SQL_NO_DATA ) { stmt->column_count = 0; 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) + { stmt->column_count = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); // 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; + stmt->row_count = driver_stmt->row_count; + } } // workaround for a bug in the PDO driver manager. It is fairly simple to crash the PDO driver manager with @@ -1146,6 +1161,10 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) // 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; } catch( core::CoreException& ) { diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index d4ffdda5..2102636f 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -1438,8 +1438,10 @@ struct sqlsrv_stmt : public sqlsrv_context { bool has_rows; // Has_rows is set if there are actual rows in the row set 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 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 set + int column_count; // Number of columns in the current result set obtained from SQLNumResultCols + int row_count; // Number of rows in the current result set obtained from SQLRowCount unsigned long query_timeout; // maximum allowed statement execution time zend_long buffered_query_limit; // maximum allowed memory for a buffered query (measured in KB) bool date_as_string; // false by default but the user can set this to true to retrieve datetime values as strings diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 9cac96a5..352d2ff0 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -140,6 +140,9 @@ 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 ), query_timeout( QUERY_TIMEOUT_INVALID ), date_as_string(false), format_decimals(false), // no formatting needed @@ -225,6 +228,9 @@ 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; // delete any current results if( current_results ) { @@ -821,7 +827,13 @@ bool core_sqlsrv_fetch( _Inout_ sqlsrv_stmt* stmt, _In_ SQLSMALLINT fetch_orient } // First time only if ( !stmt->fetch_called ) { - SQLSMALLINT has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); + SQLSMALLINT has_fields; + if (stmt->columns_rows_obtained) { + has_fields = stmt->column_count; + } else { + has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); + } + CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { throw core::CoreException(); } @@ -1066,10 +1078,23 @@ 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 ) { - // Use SQLNumResultCols to determine if we have rows or not. - SQLSMALLINT num_cols = core::SQLNumResultCols( stmt TSRMLS_CC ); - // use SQLRowCount to determine if there is a rows status waiting - SQLLEN rows_affected = core::SQLRowCount( stmt TSRMLS_CC ); + SQLSMALLINT num_cols, rows_affected; + if (stmt->columns_rows_obtained) + { + num_cols = stmt->column_count; + rows_affected = stmt->row_count; + } + else + { + // Use SQLNumResultCols to determine if we have rows or not + num_cols = core::SQLNumResultCols( stmt TSRMLS_CC ); + stmt->column_count = num_cols; + // 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); } diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 9d60de25..bdd1d2f8 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -1791,7 +1791,11 @@ SQLSMALLINT get_resultset_meta_data(_Inout_ sqlsrv_stmt * stmt) if (num_cols == 0) { getMetaData = true; + if (stmt->columns_rows_obtained == false) { num_cols = core::SQLNumResultCols(stmt TSRMLS_CC); + } else { + num_cols = stmt->column_count; + } } try { From e2a6ece5272fec2cf52693fc083addd8e84e6097 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 11 Apr 2019 12:11:02 -0700 Subject: [PATCH 3/8] Fixed test and error message --- source/sqlsrv/util.cpp | 6 +++--- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/sqlsrv/util.cpp b/source/sqlsrv/util.cpp index cec66c3f..dcddcdcd 100644 --- a/source/sqlsrv/util.cpp +++ b/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.", -28, false } + { IMSSP, (SQLCHAR*)"The active result for the query contains no fields, so no result set was created.", -28, false } }, { @@ -205,8 +205,8 @@ ss_error SS_ERRORS[] = { }, { - SS_SQLSRV_ERROR_ZEND_OBJECT_FAILED, - { IMSSP, (SQLCHAR*)"Failed to create an instance of class %1!s!.", -30, true } + SS_SQLSRV_ERROR_ZEND_OBJECT_FAILED, + { IMSSP, (SQLCHAR*)"Failed to create an instance of class %1!s!.", -30, true } }, { diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index 7f751324..1aaec027 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/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."; +$errorNoFields = "The active result for the query contains no fields, so no result set was created."; // Variable function gets an error message that depends on the OS function getFuncSeqError() From 847493bdff59a1a18ae7a86eedc222bf29b26f36 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 11 Apr 2019 12:33:39 -0700 Subject: [PATCH 4/8] Minor fixes --- source/pdo_sqlsrv/pdo_stmt.cpp | 8 ++++---- source/shared/core_sqlsrv.h | 6 +++--- source/sqlsrv/stmt.cpp | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 906f73fd..575fc8c6 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -600,10 +600,10 @@ int pdo_sqlsrv_stmt_execute( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) else { if (driver_stmt->columns_rows_obtained == false) { - stmt->column_count = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); + stmt->column_count = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); - // return the row count regardless if there are any rows or not - stmt->row_count = core::SQLRowCount( driver_stmt TSRMLS_CC ); + // 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; @@ -709,7 +709,7 @@ int pdo_sqlsrv_stmt_fetch( _Inout_ pdo_stmt_t *stmt, _In_ enum pdo_fetch_orienta // 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) {// || driver_stmt->cursor_type != SQL_CURSOR_FORWARD_ONLY ) { + if( driver_stmt->past_fetch_end) { stmt->row_count = core::SQLRowCount( driver_stmt TSRMLS_CC ); diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 2102636f..1c6cacad 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -1439,9 +1439,9 @@ 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 set - int column_count; // Number of columns in the current result set obtained from SQLNumResultCols - int row_count; // Number of rows in the current result set obtained from SQLRowCount + 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 zend_long buffered_query_limit; // maximum allowed memory for a buffered query (measured in KB) bool date_as_string; // false by default but the user can set this to true to retrieve datetime values as strings diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index bdd1d2f8..aa97231e 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -1792,7 +1792,7 @@ SQLSMALLINT get_resultset_meta_data(_Inout_ sqlsrv_stmt * stmt) if (num_cols == 0) { getMetaData = true; if (stmt->columns_rows_obtained == false) { - num_cols = core::SQLNumResultCols(stmt TSRMLS_CC); + num_cols = core::SQLNumResultCols(stmt TSRMLS_CC); } else { num_cols = stmt->column_count; } From cf03cbb6f7283be90f20ab609fba68ad97d27ce9 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 11 Apr 2019 12:47:16 -0700 Subject: [PATCH 5/8] Test fixes --- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 2 +- test/functional/sqlsrv/test_warning_errors3.phpt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index 1aaec027..bfea07d9 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -153,7 +153,7 @@ echo "Null result set, call next result first: #############################\n"; $stmt = sqlsrv_query($conn, "TestEmptySetProc @a='a', @b='c'"); NextResult($stmt, []); -Fetch($stmt, [$errorFuncSeq()]); +Fetch($stmt, [$errorNoFields]); // Call next_result twice in succession on a null result set echo "Null result set, call next result twice: #############################\n"; diff --git a/test/functional/sqlsrv/test_warning_errors3.phpt b/test/functional/sqlsrv/test_warning_errors3.phpt index 1b0bdc66..f0e7848c 100644 --- a/test/functional/sqlsrv/test_warning_errors3.phpt +++ b/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. - [message] => The active result for the query contains no fields. + [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. ) ) From 62738bacf3c0cf51d4c6c7adee8857b4b355ef17 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 12 Apr 2019 20:49:03 -0700 Subject: [PATCH 6/8] Addressed review comments --- source/pdo_sqlsrv/pdo_stmt.cpp | 36 +++++++++++-------- source/shared/core_sqlsrv.h | 6 +++- source/shared/core_stmt.cpp | 30 ++++++++-------- source/sqlsrv/stmt.cpp | 2 +- source/sqlsrv/util.cpp | 2 +- .../sqlsrv/sqlsrv_empty_result_error.phpt | 2 +- .../sqlsrv/test_warning_errors3.phpt | 4 +-- 7 files changed, 48 insertions(+), 34 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 575fc8c6..f8dd160c 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/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) { - stmt->row_count = core::SQLRowCount( driver_stmt TSRMLS_CC ); + 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& ) { diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 1c6cacad..347a89d7 100644 --- a/source/shared/core_sqlsrv.h +++ b/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 diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 352d2ff0..9d7f6801 100644 --- a/source/shared/core_stmt.cpp +++ b/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); diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index aa97231e..0765b7d4 100644 --- a/source/sqlsrv/stmt.cpp +++ b/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; diff --git a/source/sqlsrv/util.cpp b/source/sqlsrv/util.cpp index dcddcdcd..e4ea3472 100644 --- a/source/sqlsrv/util.cpp +++ b/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 } }, { diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index bfea07d9..92b7f0ae 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/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() diff --git a/test/functional/sqlsrv/test_warning_errors3.phpt b/test/functional/sqlsrv/test_warning_errors3.phpt index f0e7848c..1b0bdc66 100644 --- a/test/functional/sqlsrv/test_warning_errors3.phpt +++ b/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. ) ) From b0251101943a08cc43b1e499b664200c56325afc Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 12 Apr 2019 22:41:06 -0700 Subject: [PATCH 7/8] Fixed test failure --- source/pdo_sqlsrv/pdo_stmt.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index f8dd160c..e72b5dc2 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -714,13 +714,8 @@ int pdo_sqlsrv_stmt_fetch( _Inout_ pdo_stmt_t *stmt, _In_ enum pdo_fetch_orienta // 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; - } + stmt->row_count = core::SQLRowCount( driver_stmt TSRMLS_CC ); + driver_stmt->row_count = stmt->row_count; // a row_count of -1 means no rows, but we change it to 0 if( stmt->row_count == -1 ) { From ad1d990cda38af30d6cd76e3adc0be42193a1438 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Mon, 15 Apr 2019 13:48:48 -0700 Subject: [PATCH 8/8] Addressed review comments --- source/shared/core_stmt.cpp | 2 ++ source/sqlsrv/stmt.cpp | 1 + 2 files changed, 3 insertions(+) diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 9d7f6801..19c448e5 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -823,6 +823,7 @@ bool core_sqlsrv_fetch( _Inout_ sqlsrv_stmt* stmt, _In_ SQLSMALLINT fetch_orient CHECK_CUSTOM_ERROR( stmt->past_fetch_end, stmt, SQLSRV_ERROR_FETCH_PAST_END ) { throw core::CoreException(); } + // First time only if ( !stmt->fetch_called ) { SQLSMALLINT has_fields; @@ -830,6 +831,7 @@ bool core_sqlsrv_fetch( _Inout_ sqlsrv_stmt* stmt, _In_ SQLSMALLINT fetch_orient has_fields = stmt->column_count; } else { has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); + stmt->column_count = has_fields; } CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 0765b7d4..9d16d24e 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -1793,6 +1793,7 @@ SQLSMALLINT get_resultset_meta_data(_Inout_ sqlsrv_stmt * stmt) getMetaData = true; if (stmt->column_count == ACTIVE_NUM_COLS_INVALID) { num_cols = core::SQLNumResultCols(stmt TSRMLS_CC); + stmt->column_count = num_cols; } else { num_cols = stmt->column_count; }