From 7256685a4c817563639371083e7028aaa5b0207e Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Mon, 27 Mar 2017 17:49:01 -0700 Subject: [PATCH 01/18] workaround for unixODBC bug returning error when pdo::exec query with empty result set --- source/shared/core_sqlsrv.h | 11 ++++ ...o_336_pho_exec_empty_result_set_error.phpt | 53 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 55c0678b..7a46ca70 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -2040,6 +2040,17 @@ namespace core { SQLRETURN r; SQLSMALLINT num_cols; r = ::SQLNumResultCols( stmt->handle(), &num_cols ); + + // Workaround for a bug in unixODBC: after SQLExecDirect for an empty result set, + // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); + // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) + // but it should have succeeded with r=0 (SQL_SUCCESS) and no error + // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error + // (HY010 error should not return if stmt->execute is true) +#ifndef _WIN32 + if ( r == -1 && stmt->execute && strcmp( reinterpret_cast( stmt->last_error()->sqlstate ), "HY010" ) == 0 ) + return 0; +#endif // !_WIN32 CHECK_SQL_ERROR_OR_WARNING( r, stmt ) { throw CoreException(); diff --git a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt new file mode 100644 index 00000000..f9b2017d --- /dev/null +++ b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt @@ -0,0 +1,53 @@ +--TEST-- +GitHub issue #336 - PDO::exec should not return an error with query returning SQL_NO_DATA +--DESCRIPTION-- +Verifies GitHub issue 336 is fixed, PDO::exec on query returning SQL_NO_DATA will not give an error +--SKIPIF-- +--FILE-- +setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION ); + +$sql = "DELETE FROM foo_table WHERE id = 42"; +$sqlWithParameter = "DELETE FROM foo_table WHERE id = :id"; +$sqlParameter = 42; + +$Statement = $conn->exec("IF OBJECT_ID('foo_table', 'U') IS NOT NULL DROP TABLE foo_table"); +$Statement = $conn->exec("CREATE TABLE foo_table (id BIGINT PRIMARY KEY NOT NULL IDENTITY, intField INT NOT NULL)"); +$Statement = $conn->exec("INSERT INTO foo_table (intField) VALUES(3)"); + +//test prepare, not args +$stmt = $conn->prepare($sql); +$stmt->execute(); +if ($conn->errorCode() == 00000) + echo "prepare OK\n"; + +//test prepare, with args +$stmt = $conn->prepare($sqlWithParameter); +$stmt->execute(array(':id' => $sqlParameter)); +if ($conn->errorCode() == 00000) + echo "prepare with args OK\n"; + +//test direct exec +$stmt = $conn->exec($sql); +if ($conn->errorCode() == 00000) + echo "direct exec OK\n"; + +$Statement = $conn->exec("IF OBJECT_ID('foo_table', 'U') IS NOT NULL DROP TABLE foo_table"); + +$stmt = NULL; +$Statement = NULL; +$conn = NULL; + +?> +--EXPECT-- +prepare OK +prepare with args OK +direct exec OK \ No newline at end of file From 7c4efbcad8817a42001f8636f1f467e8db1a8cd8 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Tue, 28 Mar 2017 09:29:57 -0700 Subject: [PATCH 02/18] Update core_sqlsrv.h --- source/shared/core_sqlsrv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 7a46ca70..af72ce7e 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -2048,7 +2048,7 @@ namespace core { // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error // (HY010 error should not return if stmt->execute is true) #ifndef _WIN32 - if ( r == -1 && stmt->execute && strcmp( reinterpret_cast( stmt->last_error()->sqlstate ), "HY010" ) == 0 ) + if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( stmt->last_error()->sqlstate ), "HY010" ) == 0 ) return 0; #endif // !_WIN32 From 2788965d57fb23302f348ae4d8791543ba6c3a1f Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Tue, 28 Mar 2017 09:38:15 -0700 Subject: [PATCH 03/18] Update pdo_336_pho_exec_empty_result_set_error.phpt --- test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt index f9b2017d..5cdf012e 100644 --- a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt +++ b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt @@ -50,4 +50,4 @@ $conn = NULL; --EXPECT-- prepare OK prepare with args OK -direct exec OK \ No newline at end of file +direct exec OK From 4284457b350c4ca60d19e0fa9c359f60c17df145 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Tue, 28 Mar 2017 10:59:04 -0700 Subject: [PATCH 04/18] fix sqlsrv_statement_cancel.phpt test --- test/sqlsrv/sqlsrv_statement_cancel.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/sqlsrv/sqlsrv_statement_cancel.phpt b/test/sqlsrv/sqlsrv_statement_cancel.phpt index 4efaed9e..48fee5f0 100644 --- a/test/sqlsrv/sqlsrv_statement_cancel.phpt +++ b/test/sqlsrv/sqlsrv_statement_cancel.phpt @@ -97,7 +97,7 @@ Repro(); --EXPECTREGEX--  ...Starting 'sqlsrv_statement_cancel' test... -\[Microsoft\](\[ODBC Driver 13 for SQL Server\]|\[ODBC Driver Manager\])( Function sequence error|Associated statement is not prepared) +\[Microsoft\](\[ODBC Driver 13 for SQL Server\]|\[ODBC Driver Manager\])(Function sequence error|Associated statement is not prepared) 0 (HY010|HY007) From 7230e307a7a19f39d7dda4f4cccffcf4e676c86e Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Tue, 28 Mar 2017 11:41:35 -0700 Subject: [PATCH 05/18] Update sqlsrv_statement_cancel.phpt --- test/sqlsrv/sqlsrv_statement_cancel.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/sqlsrv/sqlsrv_statement_cancel.phpt b/test/sqlsrv/sqlsrv_statement_cancel.phpt index 48fee5f0..4efaed9e 100644 --- a/test/sqlsrv/sqlsrv_statement_cancel.phpt +++ b/test/sqlsrv/sqlsrv_statement_cancel.phpt @@ -97,7 +97,7 @@ Repro(); --EXPECTREGEX--  ...Starting 'sqlsrv_statement_cancel' test... -\[Microsoft\](\[ODBC Driver 13 for SQL Server\]|\[ODBC Driver Manager\])(Function sequence error|Associated statement is not prepared) +\[Microsoft\](\[ODBC Driver 13 for SQL Server\]|\[ODBC Driver Manager\])( Function sequence error|Associated statement is not prepared) 0 (HY010|HY007) From 53c280fe54b525dd02f37d895c98b163354055bf Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Tue, 28 Mar 2017 16:37:40 -0700 Subject: [PATCH 06/18] get error without recording it --- source/shared/core_sqlsrv.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index af72ce7e..7f2dfc38 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -2041,17 +2041,20 @@ namespace core { SQLSMALLINT num_cols; r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // Workaround for a bug in unixODBC: after SQLExecDirect for an empty result set, + // Workaround for a bug in unixODBC: after SQLExecDirect returns SQL_NO_DATA, // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) // but it should have succeeded with r=0 (SQL_SUCCESS) and no error // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error // (HY010 error should not return if stmt->execute is true) #ifndef _WIN32 - if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( stmt->last_error()->sqlstate ), "HY010" ) == 0 ) + sqlsrv_error_auto_ptr error; + if ( stmt->current_results != NULL ) { + error = stmt->current_results->get_diag_rec( 1 ); + } + if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) return 0; #endif // !_WIN32 - CHECK_SQL_ERROR_OR_WARNING( r, stmt ) { throw CoreException(); } From a6d55d8841e6fbaac62f5059aed94dfcad142600 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Tue, 28 Mar 2017 17:04:44 -0700 Subject: [PATCH 07/18] Update core_sqlsrv.h --- source/shared/core_sqlsrv.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 7f2dfc38..283677fe 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -2041,19 +2041,19 @@ namespace core { SQLSMALLINT num_cols; r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // Workaround for a bug in unixODBC: after SQLExecDirect returns SQL_NO_DATA, - // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) - // but it should have succeeded with r=0 (SQL_SUCCESS) and no error - // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error - // (HY010 error should not return if stmt->execute is true) + // Workaround for a bug in unixODBC: after SQLExecDirect returns SQL_NO_DATA, + // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); + // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) + // but it should have succeeded with r=0 (SQL_SUCCESS) and no error + // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error + // (HY010 error should not return if stmt->execute is true) #ifndef _WIN32 - sqlsrv_error_auto_ptr error; - if ( stmt->current_results != NULL ) { - error = stmt->current_results->get_diag_rec( 1 ); - } - if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) - return 0; + sqlsrv_error_auto_ptr error; + if ( stmt->current_results != NULL ) { + error = stmt->current_results->get_diag_rec( 1 ); + } + if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) + return 0; #endif // !_WIN32 CHECK_SQL_ERROR_OR_WARNING( r, stmt ) { throw CoreException(); From e90d67a0dd74c53168a87c470c52228992aa0647 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Tue, 28 Mar 2017 17:06:17 -0700 Subject: [PATCH 08/18] Update core_sqlsrv.h --- source/shared/core_sqlsrv.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 283677fe..5281ef61 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -2041,19 +2041,19 @@ namespace core { SQLSMALLINT num_cols; r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // Workaround for a bug in unixODBC: after SQLExecDirect returns SQL_NO_DATA, - // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) - // but it should have succeeded with r=0 (SQL_SUCCESS) and no error - // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error - // (HY010 error should not return if stmt->execute is true) + // Workaround for a bug in unixODBC: after SQLExecDirect returns SQL_NO_DATA, + // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); + // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) + // but it should have succeeded with r=0 (SQL_SUCCESS) and no error + // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error + // (HY010 error should not return if stmt->execute is true) #ifndef _WIN32 - sqlsrv_error_auto_ptr error; - if ( stmt->current_results != NULL ) { - error = stmt->current_results->get_diag_rec( 1 ); - } - if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) - return 0; + sqlsrv_error_auto_ptr error; + if ( stmt->current_results != NULL ) { + error = stmt->current_results->get_diag_rec( 1 ); + } + if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) + return 0; #endif // !_WIN32 CHECK_SQL_ERROR_OR_WARNING( r, stmt ) { throw CoreException(); From 0fd7ebb6d3dc10dbc1d1b5c86a18ac5a347d42e3 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Wed, 29 Mar 2017 11:26:45 -0700 Subject: [PATCH 09/18] fix seg fault --- source/shared/core_sqlsrv.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 7f2dfc38..c2a47838 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -2041,19 +2041,19 @@ namespace core { SQLSMALLINT num_cols; r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // Workaround for a bug in unixODBC: after SQLExecDirect returns SQL_NO_DATA, - // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) - // but it should have succeeded with r=0 (SQL_SUCCESS) and no error - // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error - // (HY010 error should not return if stmt->execute is true) + // Workaround for a bug in unixODBC: after SQLExecDirect returns SQL_NO_DATA, + // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); + // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) + // but it should have succeeded with r=0 (SQL_SUCCESS) and no error + // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error + // (HY010 error should not return if stmt->execute is true) #ifndef _WIN32 - sqlsrv_error_auto_ptr error; - if ( stmt->current_results != NULL ) { - error = stmt->current_results->get_diag_rec( 1 ); - } - if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) - return 0; + sqlsrv_error_auto_ptr error; + if ( stmt->current_results != NULL ) { + error = stmt->current_results->get_diag_rec( 1 ); + if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) + return 0; + } #endif // !_WIN32 CHECK_SQL_ERROR_OR_WARNING( r, stmt ) { throw CoreException(); From 816856cbc0c906a2db106b65b12087cbe5eebb35 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Wed, 29 Mar 2017 13:45:57 -0700 Subject: [PATCH 10/18] cleaner code --- source/shared/core_sqlsrv.h | 6 +++--- .../pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index c2a47838..dec5d8e6 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -2048,10 +2048,10 @@ namespace core { // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error // (HY010 error should not return if stmt->execute is true) #ifndef _WIN32 - sqlsrv_error_auto_ptr error; - if ( stmt->current_results != NULL ) { + if ( r == -1 && stmt->executed && stmt->current_results != NULL ) { + sqlsrv_error_auto_ptr error; error = stmt->current_results->get_diag_rec( 1 ); - if ( r == -1 && stmt->executed && strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) + if ( strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) return 0; } #endif // !_WIN32 diff --git a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt index 5cdf012e..52e8c13f 100644 --- a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt +++ b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt @@ -26,18 +26,18 @@ $Statement = $conn->exec("INSERT INTO foo_table (intField) VALUES(3)"); //test prepare, not args $stmt = $conn->prepare($sql); $stmt->execute(); -if ($conn->errorCode() == 00000) +if ($conn->errorCode() == "00000") echo "prepare OK\n"; //test prepare, with args $stmt = $conn->prepare($sqlWithParameter); $stmt->execute(array(':id' => $sqlParameter)); -if ($conn->errorCode() == 00000) +if ($conn->errorCode() == "00000") echo "prepare with args OK\n"; //test direct exec $stmt = $conn->exec($sql); -if ($conn->errorCode() == 00000) +if ($conn->errorCode() == "00000") echo "direct exec OK\n"; $Statement = $conn->exec("IF OBJECT_ID('foo_table', 'U') IS NOT NULL DROP TABLE foo_table"); From a5f999ef0ba0f9ff6448fe49219af66268961b02 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Wed, 29 Mar 2017 13:48:11 -0700 Subject: [PATCH 11/18] cleaner code --- test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt index 52e8c13f..ae6d1d45 100644 --- a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt +++ b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt @@ -5,8 +5,6 @@ Verifies GitHub issue 336 is fixed, PDO::exec on query returning SQL_NO_DATA wil --SKIPIF-- --FILE-- Date: Thu, 30 Mar 2017 13:17:50 -0700 Subject: [PATCH 12/18] run again --- test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt | 1 - 1 file changed, 1 deletion(-) diff --git a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt index ae6d1d45..322b9ba9 100644 --- a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt +++ b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt @@ -7,7 +7,6 @@ Verifies GitHub issue 336 is fixed, PDO::exec on query returning SQL_NO_DATA wil Date: Fri, 31 Mar 2017 13:38:57 -0700 Subject: [PATCH 13/18] do not check has result if execute returns SQL_NO_DATA --- source/pdo_sqlsrv/pdo_dbh.cpp | 4 ++-- source/pdo_sqlsrv/pdo_stmt.cpp | 14 ++++++++++---- source/shared/core_sqlsrv.h | 17 ++--------------- source/shared/core_stmt.cpp | 3 ++- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_dbh.cpp b/source/pdo_sqlsrv/pdo_dbh.cpp index 5323058f..c7b7d967 100644 --- a/source/pdo_sqlsrv/pdo_dbh.cpp +++ b/source/pdo_sqlsrv/pdo_dbh.cpp @@ -690,11 +690,11 @@ zend_long pdo_sqlsrv_dbh_do( pdo_dbh_t *dbh, const char *sql, size_t sql_len TSR NULL /*valid_stmt_opts*/, pdo_sqlsrv_handle_stmt_error, &temp_stmt TSRMLS_CC ); driver_stmt->set_func( __FUNCTION__ ); - core_sqlsrv_execute( driver_stmt TSRMLS_CC, sql, static_cast( sql_len ) ); + SQLRETURN execReturn = core_sqlsrv_execute( driver_stmt TSRMLS_CC, sql, static_cast( sql_len ) ); // since the user can give us a compound statement, we return the row count for the last set, and since the row count // isn't guaranteed to be valid until all the results have been fetched, we fetch them all first. - if( core_sqlsrv_has_any_result( driver_stmt TSRMLS_CC )) { + if( execReturn != SQL_NO_DATA && core_sqlsrv_has_any_result( driver_stmt TSRMLS_CC )) { SQLRETURN r = SQL_SUCCESS; diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 2506960b..bfe25cca 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -552,12 +552,18 @@ int pdo_sqlsrv_stmt_execute(pdo_stmt_t *stmt TSRMLS_DC) query_len = static_cast( stmt->active_query_stringlen ); } - core_sqlsrv_execute( driver_stmt TSRMLS_CC, query, query_len ); + SQLRETURN execReturn = core_sqlsrv_execute( driver_stmt TSRMLS_CC, query, query_len ); - stmt->column_count = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); + if ( execReturn == SQL_NO_DATA ) { + stmt->column_count = 0; + stmt->row_count = 0; + } + else { + 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 ); + } // workaround for a bug in the PDO driver manager. It is fairly simple to crash the PDO driver manager with // the following sequence: diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index dec5d8e6..530d41a6 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -1363,7 +1363,7 @@ sqlsrv_stmt* core_sqlsrv_create_stmt( sqlsrv_conn* conn, driver_stmt_factory stm void core_sqlsrv_bind_param( sqlsrv_stmt* stmt, SQLUSMALLINT param_num, SQLSMALLINT direction, zval* param_z, SQLSRV_PHPTYPE php_out_type, SQLSRV_ENCODING encoding, SQLSMALLINT sql_type, SQLULEN column_size, SQLSMALLINT decimal_digits TSRMLS_DC ); -void core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql = NULL, int sql_len = 0 ); +SQLRETURN core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql = NULL, int sql_len = 0 ); field_meta_data* core_sqlsrv_field_metadata( sqlsrv_stmt* stmt, SQLSMALLINT colno TSRMLS_DC ); bool core_sqlsrv_fetch( sqlsrv_stmt* stmt, SQLSMALLINT fetch_orientation, SQLULEN fetch_offset TSRMLS_DC ); void core_sqlsrv_get_field(sqlsrv_stmt* stmt, SQLUSMALLINT field_index, sqlsrv_phptype sqlsrv_phptype, bool prefer_string, @@ -2041,20 +2041,7 @@ namespace core { SQLSMALLINT num_cols; r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // Workaround for a bug in unixODBC: after SQLExecDirect returns SQL_NO_DATA, - // r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - // returns r=-1 (SQL_ERROR) and error HY010 (Function sequence error) - // but it should have succeeded with r=0 (SQL_SUCCESS) and no error - // instead of throwing an exception, return 0 if the r=-1, stament has been executed, and has a HY010 error - // (HY010 error should not return if stmt->execute is true) -#ifndef _WIN32 - if ( r == -1 && stmt->executed && stmt->current_results != NULL ) { - sqlsrv_error_auto_ptr error; - error = stmt->current_results->get_diag_rec( 1 ); - if ( strcmp( reinterpret_cast( error->sqlstate ), "HY010" ) == 0 ) - return 0; - } -#endif // !_WIN32 + CHECK_SQL_ERROR_OR_WARNING( r, stmt ) { throw CoreException(); } diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 9dd505ec..53605004 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -657,7 +657,7 @@ void core_sqlsrv_bind_param( sqlsrv_stmt* stmt, SQLUSMALLINT param_num, SQLSMALL // Return: // true if there is data, false if there is not -void core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql, int sql_len ) +SQLRETURN core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql, int sql_len ) { SQLRETURN r; @@ -708,6 +708,7 @@ void core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql, int sql_ if ( stmt->send_streams_at_exec ) { zend_hash_clean( Z_ARRVAL( stmt->param_streams )); } + return r; } catch( core::CoreException& e ) { From 9d033e716ad5836bee1c33dff82708010306ee44 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Fri, 31 Mar 2017 13:59:30 -0700 Subject: [PATCH 14/18] added more to test --- source/shared/core_sqlsrv.h | 1 - .../pdo_336_pho_exec_empty_result_set_error.phpt | 12 +++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/source/shared/core_sqlsrv.h b/source/shared/core_sqlsrv.h index 530d41a6..cfb1efc2 100644 --- a/source/shared/core_sqlsrv.h +++ b/source/shared/core_sqlsrv.h @@ -2041,7 +2041,6 @@ namespace core { SQLSMALLINT num_cols; r = ::SQLNumResultCols( stmt->handle(), &num_cols ); - CHECK_SQL_ERROR_OR_WARNING( r, stmt ) { throw CoreException(); } diff --git a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt index 322b9ba9..9a6bb61f 100644 --- a/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt +++ b/test/pdo_sqlsrv/pdo_336_pho_exec_empty_result_set_error.phpt @@ -25,17 +25,27 @@ $stmt = $conn->prepare($sql); $stmt->execute(); if ($conn->errorCode() == "00000") echo "prepare OK\n"; +else + echo "unexpected error at prepare"; //test prepare, with args $stmt = $conn->prepare($sqlWithParameter); $stmt->execute(array(':id' => $sqlParameter)); if ($conn->errorCode() == "00000") echo "prepare with args OK\n"; +else + echo "unexpected error at prepare with args"; //test direct exec $stmt = $conn->exec($sql); -if ($conn->errorCode() == "00000") +$err = $conn->errorCode(); +if ($stmt == 0 && $err == "00000") echo "direct exec OK\n"; +else + if ($stmt != 0) + echo "unexpected row returned at direct exec\n"; + if ($err != "00000") + echo "unexpected error at direct exec"; $Statement = $conn->exec("IF OBJECT_ID('foo_table', 'U') IS NOT NULL DROP TABLE foo_table"); From 20c4fe69c1f34afd5205ec473edd15cea0f8ce8a Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Fri, 31 Mar 2017 15:48:22 -0700 Subject: [PATCH 15/18] fix if sqlreturn is null --- source/pdo_sqlsrv/pdo_dbh.cpp | 21 ++++++++++++--------- source/pdo_sqlsrv/pdo_stmt.cpp | 18 ++++++++++-------- source/shared/core_stmt.cpp | 24 ++++++++++++------------ 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_dbh.cpp b/source/pdo_sqlsrv/pdo_dbh.cpp index c7b7d967..b3db945f 100644 --- a/source/pdo_sqlsrv/pdo_dbh.cpp +++ b/source/pdo_sqlsrv/pdo_dbh.cpp @@ -694,18 +694,21 @@ zend_long pdo_sqlsrv_dbh_do( pdo_dbh_t *dbh, const char *sql, size_t sql_len TSR // since the user can give us a compound statement, we return the row count for the last set, and since the row count // isn't guaranteed to be valid until all the results have been fetched, we fetch them all first. - if( execReturn != SQL_NO_DATA && core_sqlsrv_has_any_result( driver_stmt TSRMLS_CC )) { - SQLRETURN r = SQL_SUCCESS; + if ( execReturn ) { + if ( execReturn != SQL_NO_DATA && core_sqlsrv_has_any_result( driver_stmt TSRMLS_CC )) { - do { - - rows = core::SQLRowCount( driver_stmt TSRMLS_CC ); + SQLRETURN r = SQL_SUCCESS; - r = core::SQLMoreResults( driver_stmt TSRMLS_CC ); - - } while( r != SQL_NO_DATA ); - } + do { + + rows = core::SQLRowCount( driver_stmt TSRMLS_CC ); + + r = core::SQLMoreResults( driver_stmt TSRMLS_CC ); + + } while ( r != SQL_NO_DATA ); + } + } // returning -1 forces PDO to return false, which signals an error occurred. SQLRowCount returns -1 for a number of cases // naturally, so we override that here with no rows returned. diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index bfe25cca..dd508a33 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -554,15 +554,17 @@ int pdo_sqlsrv_stmt_execute(pdo_stmt_t *stmt TSRMLS_DC) SQLRETURN execReturn = core_sqlsrv_execute( driver_stmt TSRMLS_CC, query, query_len ); - if ( execReturn == SQL_NO_DATA ) { - stmt->column_count = 0; - stmt->row_count = 0; - } - else { - stmt->column_count = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); + if ( execReturn ) { + if ( execReturn == SQL_NO_DATA ) { + stmt->column_count = 0; + stmt->row_count = 0; + } + else { + 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 ); + } } // workaround for a bug in the PDO driver manager. It is fairly simple to crash the PDO driver manager with diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 53605004..d161c662 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -659,7 +659,7 @@ void core_sqlsrv_bind_param( sqlsrv_stmt* stmt, SQLUSMALLINT param_num, SQLSMALL SQLRETURN core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql, int sql_len ) { - SQLRETURN r; + SQLRETURN r = NULL; try { @@ -690,7 +690,7 @@ SQLRETURN core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql, int r = core::SQLExecute( stmt TSRMLS_CC ); } - // if data is needed (streams were bound) and they should be sent at execute time, then do so now + // if data is needed (streams were bound) and they should be sent at execute time, then do so now if( r == SQL_NEED_DATA && stmt->send_streams_at_exec ) { send_param_streams( stmt TSRMLS_CC ); @@ -704,21 +704,21 @@ SQLRETURN core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql, int finalize_output_parameters( stmt TSRMLS_CC ); } - // stream parameters are sent, clean the Hashtable - if ( stmt->send_streams_at_exec ) { - zend_hash_clean( Z_ARRVAL( stmt->param_streams )); - } - return r; + // stream parameters are sent, clean the Hashtable + if ( stmt->send_streams_at_exec ) { + zend_hash_clean( Z_ARRVAL( stmt->param_streams )); + } + return r; } catch( core::CoreException& e ) { // if the statement executed but failed in a subsequent operation before returning, // we need to cancel the statement and deref the output and stream parameters - if ( stmt->send_streams_at_exec ) { - zend_hash_clean( Z_ARRVAL( stmt->output_params )); - zend_hash_clean( Z_ARRVAL( stmt->param_streams )); - } - if( stmt->executed ) { + if ( stmt->send_streams_at_exec ) { + zend_hash_clean( Z_ARRVAL( stmt->output_params )); + zend_hash_clean( Z_ARRVAL( stmt->param_streams )); + } + if( stmt->executed ) { SQLCancel( stmt->handle() ); // stmt->executed = false; should this be reset if something fails? } From 8f67221e18576db3f1d8165f53ef8b844a2cdaf6 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Mon, 3 Apr 2017 10:08:36 -0700 Subject: [PATCH 16/18] fix indentation --- source/pdo_sqlsrv/pdo_dbh.cpp | 18 +++++++++--------- source/pdo_sqlsrv/pdo_stmt.cpp | 22 +++++++++++----------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_dbh.cpp b/source/pdo_sqlsrv/pdo_dbh.cpp index b3db945f..9c2b9342 100644 --- a/source/pdo_sqlsrv/pdo_dbh.cpp +++ b/source/pdo_sqlsrv/pdo_dbh.cpp @@ -695,20 +695,20 @@ zend_long pdo_sqlsrv_dbh_do( pdo_dbh_t *dbh, const char *sql, size_t sql_len TSR // since the user can give us a compound statement, we return the row count for the last set, and since the row count // isn't guaranteed to be valid until all the results have been fetched, we fetch them all first. - if ( execReturn ) { - if ( execReturn != SQL_NO_DATA && core_sqlsrv_has_any_result( driver_stmt TSRMLS_CC )) { + if ( execReturn ) { + if ( execReturn != SQL_NO_DATA && core_sqlsrv_has_any_result( driver_stmt TSRMLS_CC )) { - SQLRETURN r = SQL_SUCCESS; + SQLRETURN r = SQL_SUCCESS; - do { + do { - rows = core::SQLRowCount( driver_stmt TSRMLS_CC ); + rows = core::SQLRowCount( driver_stmt TSRMLS_CC ); - r = core::SQLMoreResults( driver_stmt TSRMLS_CC ); + r = core::SQLMoreResults( driver_stmt TSRMLS_CC ); - } while ( r != SQL_NO_DATA ); - } - } + } while ( r != SQL_NO_DATA ); + } + } // returning -1 forces PDO to return false, which signals an error occurred. SQLRowCount returns -1 for a number of cases // naturally, so we override that here with no rows returned. diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index dd508a33..9fe4b451 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -554,18 +554,18 @@ int pdo_sqlsrv_stmt_execute(pdo_stmt_t *stmt TSRMLS_DC) SQLRETURN execReturn = core_sqlsrv_execute( driver_stmt TSRMLS_CC, query, query_len ); - if ( execReturn ) { - if ( execReturn == SQL_NO_DATA ) { - stmt->column_count = 0; - stmt->row_count = 0; - } - else { - stmt->column_count = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); + if ( execReturn ) { + if ( execReturn == SQL_NO_DATA ) { + stmt->column_count = 0; + stmt->row_count = 0; + } + else { + 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 ); + } + } // workaround for a bug in the PDO driver manager. It is fairly simple to crash the PDO driver manager with // the following sequence: From ff3a23f442a17099bcec9264511a98cb272d05ba Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Mon, 3 Apr 2017 12:43:08 -0700 Subject: [PATCH 17/18] got rid of execReturn check since SQL_SUCCESS is 0 --- source/pdo_sqlsrv/pdo_dbh.cpp | 14 ++++++-------- source/pdo_sqlsrv/pdo_stmt.cpp | 18 ++++++++---------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_dbh.cpp b/source/pdo_sqlsrv/pdo_dbh.cpp index 9c2b9342..02334b67 100644 --- a/source/pdo_sqlsrv/pdo_dbh.cpp +++ b/source/pdo_sqlsrv/pdo_dbh.cpp @@ -695,19 +695,17 @@ zend_long pdo_sqlsrv_dbh_do( pdo_dbh_t *dbh, const char *sql, size_t sql_len TSR // since the user can give us a compound statement, we return the row count for the last set, and since the row count // isn't guaranteed to be valid until all the results have been fetched, we fetch them all first. - if ( execReturn ) { - if ( execReturn != SQL_NO_DATA && core_sqlsrv_has_any_result( driver_stmt TSRMLS_CC )) { + if ( execReturn != SQL_NO_DATA && core_sqlsrv_has_any_result( driver_stmt TSRMLS_CC )) { - SQLRETURN r = SQL_SUCCESS; + SQLRETURN r = SQL_SUCCESS; - do { + do { - rows = core::SQLRowCount( driver_stmt TSRMLS_CC ); + rows = core::SQLRowCount( driver_stmt TSRMLS_CC ); - r = core::SQLMoreResults( driver_stmt TSRMLS_CC ); + r = core::SQLMoreResults( driver_stmt TSRMLS_CC ); - } while ( r != SQL_NO_DATA ); - } + } while ( r != SQL_NO_DATA ); } // returning -1 forces PDO to return false, which signals an error occurred. SQLRowCount returns -1 for a number of cases diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 9fe4b451..6d117063 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -554,17 +554,15 @@ int pdo_sqlsrv_stmt_execute(pdo_stmt_t *stmt TSRMLS_DC) SQLRETURN execReturn = core_sqlsrv_execute( driver_stmt TSRMLS_CC, query, query_len ); - if ( execReturn ) { - if ( execReturn == SQL_NO_DATA ) { - stmt->column_count = 0; - stmt->row_count = 0; - } - else { - stmt->column_count = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); + if ( execReturn == SQL_NO_DATA ) { + stmt->column_count = 0; + stmt->row_count = 0; + } + else { + 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 ); } // workaround for a bug in the PDO driver manager. It is fairly simple to crash the PDO driver manager with From df3e23af45f77dfa1a5b43500eeddd79b7b2bed5 Mon Sep 17 00:00:00 2001 From: v-kaywon Date: Tue, 4 Apr 2017 11:10:56 -0700 Subject: [PATCH 18/18] changed default execReturn to SQL_ERROR --- source/shared/core_stmt.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index d161c662..e44f3d6c 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -659,7 +659,7 @@ void core_sqlsrv_bind_param( sqlsrv_stmt* stmt, SQLUSMALLINT param_num, SQLSMALL SQLRETURN core_sqlsrv_execute( sqlsrv_stmt* stmt TSRMLS_DC, const char* sql, int sql_len ) { - SQLRETURN r = NULL; + SQLRETURN r = SQL_ERROR; try {