From 83631cdabcaf34a1ac8edc21ead4ccfd2c84433b Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 7 Sep 2017 15:44:43 -0700 Subject: [PATCH 01/27] Added empty result set check to nextRowset() --- source/pdo_sqlsrv/pdo_stmt.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 85d726fd5..d548d6bcb 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,6 +1067,14 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); + // If SQLNumResultCols returns 0, the result set is empty. Normally this error is + // handled in core_sqlsrv_fetch, but if the user calls nextRowset() before fetch() + // the error message won't show up unless we handle it here. + SQLSMALLINT has_fields = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); + CHECK_CUSTOM_ERROR( has_fields == 0, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + core_sqlsrv_next_result( static_cast( stmt->driver_data ) TSRMLS_CC ); // clear the current meta data since the new result will generate new meta data From f4117a88d47071afaceb35be68138c2b82505385 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 7 Sep 2017 19:27:45 -0700 Subject: [PATCH 02/27] Added PHPT test --- .../pdo_sqlsrv/pdo_empty_result_error.phpt | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt diff --git a/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt new file mode 100644 index 000000000..660ff5062 --- /dev/null +++ b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt @@ -0,0 +1,96 @@ +--TEST-- +Test that calling nextRowset() on an empty result set produces the correct error message. Fix for Github 507. +--SKIPIF-- + +--FILE-- +setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION ); + +$stmt = $conn->query("IF OBJECT_ID('TestEmptySetTable', 'U') IS NOT NULL DROP TABLE TestEmptySetTable"); +$stmt = $conn->query("CREATE TABLE TestEmptySetTable ([c1] nvarchar(10),[c2] nvarchar(10))"); + +$stmt = $conn->query("INSERT INTO TestEmptySetTable (c1, c2) VALUES ('a', 'b')"); +$stmt = $conn->query("IF OBJECT_ID('TestEmptySetProc', 'P') IS NOT NULL DROP PROCEDURE TestEmptySetProc"); +$stmt = $conn->query("CREATE PROCEDURE TestEmptySetProc @a nvarchar(10), @b nvarchar(10) + AS SET NOCOUNT ON + BEGIN + IF @b='b' + BEGIN + SELECT 'a' as testValue + END + ELSE + BEGIN + UPDATE TestEmptySetTable SET c2 = 'c' WHERE c1 = @a + END + END"); + +// errors out when reaching the second nextRowset() call +// returned error indicates there are no more results +echo "Return a nonempty result set:\n"; +try +{ + $stmt = $conn->query("TestEmptySetProc @a='a', @b='b'"); + $result = $stmt->fetchAll(); + print_r($result); + $stmt->nextRowset(); + $result = $stmt->fetchAll(); + print_r($result); + $stmt->nextRowset(); +} +catch(Exception $e) +{ + echo $e->getMessage()."\n"; +} + +// errors out indicating the result set contains no fields +echo "Return an empty result set, call nextRowset on it before fetching anything:\n"; +try +{ + $stmt = $conn->query("TestEmptySetProc @a='a', @b='c'"); + $stmt->nextRowset(); +} +catch(Exception $e) +{ + echo $e->getMessage()."\n"; +} + +// errors out indicating the result set contains no fields +echo "Return an empty result set, call fetch on it:\n"; +try +{ + $stmt = $conn->query("TestEmptySetProc @a='a', @b='c'"); + $result = $stmt->fetchAll(); + print_r($result); +} +catch(Exception $e) +{ + echo $e->getMessage()."\n"; +} + +$stmt = $conn->query("DROP TABLE TestEmptySetTable"); +$stmt = $conn->query("DROP PROCEDURE TestEmptySetProc"); + +$conn = null; +?> +--EXPECT-- +Return a nonempty result set: +Array +( + [0] => Array + ( + [testValue] => a + [0] => a + ) + +) +Array +( +) +SQLSTATE[IMSSP]: There are no more results returned by the query. +Return an empty result set, call nextRowset on it before fetching anything: +SQLSTATE[IMSSP]: The active result for the query contains no fields. +Return an empty result set, call fetch on it: +SQLSTATE[IMSSP]: The active result for the query contains no fields. From 8666ea63c71dfc16a178baef5e9500c6575b31b9 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 21 Sep 2017 17:55:59 -0700 Subject: [PATCH 03/27] Fixed bug for sqlsrv too, sqlsrv test added --- source/pdo_sqlsrv/pdo_stmt.cpp | 8 -- source/shared/core_stmt.cpp | 9 ++ .../sqlsrv/sqlsrv_empty_result_error.phpt | 118 ++++++++++++++++++ 3 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 test/functional/sqlsrv/sqlsrv_empty_result_error.phpt diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index d548d6bcb..85d726fd5 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,14 +1067,6 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); - // If SQLNumResultCols returns 0, the result set is empty. Normally this error is - // handled in core_sqlsrv_fetch, but if the user calls nextRowset() before fetch() - // the error message won't show up unless we handle it here. - SQLSMALLINT has_fields = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); - CHECK_CUSTOM_ERROR( has_fields == 0, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { - throw core::CoreException(); - } - core_sqlsrv_next_result( static_cast( stmt->driver_data ) TSRMLS_CC ); // clear the current meta data since the new result will generate new meta data diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 349dbfc02..bcb4697fd 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -1032,6 +1032,15 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC, _In_ bool fin CHECK_CUSTOM_ERROR( stmt->past_next_result_end, stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { throw core::CoreException(); } + + // Make sure that the result set is not null, i.e. SQLNumResultCols() does not + // return 0. Normally this error is handled in core_sqlsrv_fetch, but if the + // user calls sqlsrv_next_result() or nextRowset() before fetch() the error is + // never shown so we handle it here. + SQLSMALLINT has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); + CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } close_active_stream( stmt TSRMLS_CC ); diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt new file mode 100644 index 000000000..68fedaa33 --- /dev/null +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -0,0 +1,118 @@ +--TEST-- +Test that calling sqlsrv_next_result() on a null result set produces the correct error message. Fix for Github 507. +--SKIPIF-- + +--FILE-- +$databaseName, "uid"=>$uid, "pwd"=>$pwd)); + +$stmt = sqlsrv_query($conn, "IF OBJECT_ID('TestEmptySetTable', 'U') IS NOT NULL DROP TABLE TestEmptySetTable"); +$stmt = sqlsrv_query($conn, "CREATE TABLE TestEmptySetTable ([c1] nvarchar(10),[c2] nvarchar(10))"); +$stmt = sqlsrv_query($conn, "INSERT INTO TestEmptySetTable (c1, c2) VALUES ('a', 'b')"); +$stmt = sqlsrv_query($conn, "IF OBJECT_ID('TestEmptySetProc', 'P') IS NOT NULL DROP PROCEDURE TestEmptySetProc"); +$stmt = sqlsrv_query($conn, "CREATE PROCEDURE TestEmptySetProc @a nvarchar(10), @b nvarchar(10) + AS SET NOCOUNT ON + BEGIN + IF @b='b' + BEGIN + SELECT 'a' as testValue + END + ELSE + BEGIN + UPDATE TestEmptySetTable SET c2 = 'c' WHERE c1 = @a + END + END"); + +// errors out when reaching the second nextRowset() call +// returned error indicates there are no more results +echo "Return a nonempty result set:\n"; + +$stmt = sqlsrv_query($conn,"TestEmptySetProc @a='a', @b='b'"); +$result = sqlsrv_fetch_array($stmt); +print_r($result); +sqlsrv_next_result($stmt); +$result = sqlsrv_fetch_array($stmt); +print_r($result); +sqlsrv_next_result($stmt); + +print_r(sqlsrv_errors()); + +// errors out indicating the result set contains no fields +echo "Return an empty result set, call nextRowset on it before fetching anything:\n"; + +$stmt = sqlsrv_query($conn, "TestEmptySetProc @a='a', @b='c'"); +sqlsrv_next_result($stmt); +print_r(sqlsrv_errors()); + +// errors out indicating the result set contains no fields +echo "Return an empty result set, call fetch on it:\n"; + +$stmt = sqlsrv_query($conn, "TestEmptySetProc @a='a', @b='c'"); +$result = sqlsrv_fetch_array($stmt); +print_r($result); +print_r(sqlsrv_errors()); + +$stmt = sqlsrv_query($conn, "DROP TABLE TestEmptySetTable"); +$stmt = sqlsrv_query($conn, "DROP PROCEDURE TestEmptySetProc"); +sqlsrv_free($conn); +?> +--EXPECT-- +Return a nonempty result set: +Array +( + [0] => a + [testValue] => a +) +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -26 + [code] => -26 + [2] => There are no more results returned by the query. + [message] => There are no more results returned by the query. + ) + + [1] => Array + ( + [0] => HY010 + [SQLSTATE] => HY010 + [1] => 0 + [code] => 0 + [2] => [unixODBC][Driver Manager]Function sequence error + [message] => [unixODBC][Driver Manager]Function sequence error + ) + +) +Return an empty result set, call nextRowset on it before fetching anything: +Array +( + [0] => Array + ( + [0] => IMSSP + [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. + ) + +) +Return an empty result set, call fetch on it: +Array +( + [0] => Array + ( + [0] => IMSSP + [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. + ) + +) From 7046086f60aa2c4f65b66c6809866eced43dbdc3 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 21 Sep 2017 18:27:56 -0700 Subject: [PATCH 04/27] Fixed test failures by moving logic back to stmt.cpp and pdo_stmt.cpp --- source/pdo_sqlsrv/pdo_stmt.cpp | 8 ++++++++ source/shared/core_stmt.cpp | 9 --------- source/sqlsrv/stmt.cpp | 9 +++++++++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 85d726fd5..1a34a9b96 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,6 +1067,14 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); + // Make sure that the result set is not null, i.e. SQLNumResultCols() does not + // return 0. Normally this error is handled in core_sqlsrv_fetch, but if the + // user calls nextRowset() before fetch() the error is never shown so we handle it here. + SQLSMALLINT has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); + CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + core_sqlsrv_next_result( static_cast( stmt->driver_data ) TSRMLS_CC ); // clear the current meta data since the new result will generate new meta data diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index bcb4697fd..f4df065f5 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -1033,15 +1033,6 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC, _In_ bool fin throw core::CoreException(); } - // Make sure that the result set is not null, i.e. SQLNumResultCols() does not - // return 0. Normally this error is handled in core_sqlsrv_fetch, but if the - // user calls sqlsrv_next_result() or nextRowset() before fetch() the error is - // never shown so we handle it here. - SQLSMALLINT has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); - CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { - throw core::CoreException(); - } - close_active_stream( stmt TSRMLS_CC ); //Clear column sql types and sql display sizes. diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 015dddfc2..5251834e2 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -1006,6 +1006,15 @@ PHP_FUNCTION( sqlsrv_send_stream_data ) try { + // Make sure that the result set is not null, i.e. SQLNumResultCols() does not + // return 0. Normally this error is handled in core_sqlsrv_fetch, but if the + // user calls sqlsrv_next_result() before fetch() the error is never shown so + // we handle it here. + SQLSMALLINT has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); + CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + // if everything was sent at execute time, just return that there is nothing more to send. if( stmt->send_streams_at_exec ) { RETURN_NULL(); From 0d06dea2e8ecf06b30aca2e86d15c7f5bbadeb64 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 21 Sep 2017 18:37:23 -0700 Subject: [PATCH 05/27] Fixed compile bug --- source/pdo_sqlsrv/pdo_stmt.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 1a34a9b96..7ce2e0f24 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1070,8 +1070,8 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) // Make sure that the result set is not null, i.e. SQLNumResultCols() does not // return 0. Normally this error is handled in core_sqlsrv_fetch, but if the // user calls nextRowset() before fetch() the error is never shown so we handle it here. - SQLSMALLINT has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); - CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { + SQLSMALLINT has_fields = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); + CHECK_CUSTOM_ERROR( has_fields == 0, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { throw core::CoreException(); } From 2c28d0bf312fc856258c1aaaf23d44c8c4e83059 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 22 Sep 2017 00:13:33 -0400 Subject: [PATCH 06/27] Update stmt.cpp --- source/sqlsrv/stmt.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 5251834e2..cb7fbff69 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -560,8 +560,17 @@ PHP_FUNCTION( sqlsrv_next_result ) PROCESS_PARAMS( stmt, "r", _FN_, 0 ); try { - - core_sqlsrv_next_result( stmt TSRMLS_CC, true ); + + // Make sure that the result set is not null, i.e. SQLNumResultCols() does not + // return 0. Normally this error is handled in core_sqlsrv_fetch, but if the + // user calls sqlsrv_next_result() before fetch() the error is never shown so + // we handle it here. + SQLSMALLINT has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); + CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + + core_sqlsrv_next_result( stmt TSRMLS_CC, true ); if( stmt->past_next_result_end ) { @@ -1006,15 +1015,6 @@ PHP_FUNCTION( sqlsrv_send_stream_data ) try { - // Make sure that the result set is not null, i.e. SQLNumResultCols() does not - // return 0. Normally this error is handled in core_sqlsrv_fetch, but if the - // user calls sqlsrv_next_result() before fetch() the error is never shown so - // we handle it here. - SQLSMALLINT has_fields = core::SQLNumResultCols( stmt TSRMLS_CC ); - CHECK_CUSTOM_ERROR( has_fields == 0, stmt, SQLSRV_ERROR_NO_FIELDS ) { - throw core::CoreException(); - } - // if everything was sent at execute time, just return that there is nothing more to send. if( stmt->send_streams_at_exec ) { RETURN_NULL(); From f11c2084f28054674585f6c80534af5ec1f69a99 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 22 Sep 2017 00:33:37 -0400 Subject: [PATCH 07/27] Update sqlsrv_empty_result_error.phpt --- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index 68fedaa33..c8cb6b1d8 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -56,7 +56,7 @@ print_r(sqlsrv_errors()); $stmt = sqlsrv_query($conn, "DROP TABLE TestEmptySetTable"); $stmt = sqlsrv_query($conn, "DROP PROCEDURE TestEmptySetProc"); -sqlsrv_free($conn); +sqlsrv_close($conn); ?> --EXPECT-- Return a nonempty result set: From 270f2b446e88453c48873ed0310d01c9d260480e Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Tue, 26 Sep 2017 16:02:30 -0700 Subject: [PATCH 08/27] Use core_sqlsrv_has_any_result to check for null results --- source/pdo_sqlsrv/pdo_stmt.cpp | 16 ++++++++++------ source/sqlsrv/stmt.cpp | 12 ++++++++++++ .../sqlsrv/sqlsrv_empty_result_error.phpt | 12 +----------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 549de06fe..b74ca3609 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,12 +1067,16 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); - // Make sure that the result set is not null, i.e. SQLNumResultCols() does not - // return 0. Normally this error is handled in core_sqlsrv_fetch, but if the - // user calls nextRowset() before fetch() the error is never shown so we handle it here. - SQLSMALLINT has_fields = core::SQLNumResultCols( driver_stmt TSRMLS_CC ); - CHECK_CUSTOM_ERROR( has_fields == 0, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { - throw core::CoreException(); + // Make sure that the result set is not null. Null means SQLNumResultCols returns 0 + // and SQLRowCount is not > 0. Normally this error is handled in core_sqlsrv_fetch(), + // but if the user calls nextRowset() before fetch() the error is never shown + // so we handle it here. + bool has_result = core_sqlsrv_has_any_result( driver_stmt ); + + if(!driver_stmt->fetch_called){ + CHECK_CUSTOM_ERROR( !has_result, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } } core_sqlsrv_next_result( static_cast( stmt->driver_data ) TSRMLS_CC ); diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 57be959b3..3feb2c5b0 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -561,6 +561,18 @@ PHP_FUNCTION( sqlsrv_next_result ) try { + // Make sure that the result set is not null. Null means SQLNumResultCols returns 0 + // and SQLRowCount is not > 0. Normally this error is handled in core_sqlsrv_fetch(), + // but if the user calls sqlsrv_next_result() before fetch() the error is never shown + // so we handle it here. + bool has_result = core_sqlsrv_has_any_result( stmt ); + + if(!stmt->fetch_called){ + CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + } + core_sqlsrv_next_result( stmt TSRMLS_CC, true ); if( stmt->past_next_result_end ) { diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index 68fedaa33..f073d4e51 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -56,7 +56,7 @@ print_r(sqlsrv_errors()); $stmt = sqlsrv_query($conn, "DROP TABLE TestEmptySetTable"); $stmt = sqlsrv_query($conn, "DROP PROCEDURE TestEmptySetProc"); -sqlsrv_free($conn); +sqlsrv_close($conn); ?> --EXPECT-- Return a nonempty result set: @@ -68,16 +68,6 @@ Array Array ( [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -26 - [code] => -26 - [2] => There are no more results returned by the query. - [message] => There are no more results returned by the query. - ) - - [1] => Array ( [0] => HY010 [SQLSTATE] => HY010 From 68bb42b2cf50bf05996f5d44906b12561731e6f2 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Tue, 26 Sep 2017 18:33:49 -0700 Subject: [PATCH 09/27] Moved check to core_sqlsrv_next_result to avoid suppressing a different error message --- source/pdo_sqlsrv/pdo_stmt.cpp | 12 ------------ source/shared/core_stmt.cpp | 12 ++++++++++++ source/sqlsrv/stmt.cpp | 12 ------------ 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index b74ca3609..78828a02e 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,18 +1067,6 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); - // Make sure that the result set is not null. Null means SQLNumResultCols returns 0 - // and SQLRowCount is not > 0. Normally this error is handled in core_sqlsrv_fetch(), - // but if the user calls nextRowset() before fetch() the error is never shown - // so we handle it here. - bool has_result = core_sqlsrv_has_any_result( driver_stmt ); - - if(!driver_stmt->fetch_called){ - CHECK_CUSTOM_ERROR( !has_result, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { - throw core::CoreException(); - } - } - core_sqlsrv_next_result( static_cast( stmt->driver_data ) TSRMLS_CC ); // clear the current meta data since the new result will generate new meta data diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index 29255029b..7648ea32b 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -1058,6 +1058,18 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC, _In_ bool fin throw core::CoreException(); } + // Make sure that the result set is not null. Null means SQLNumResultCols returns 0 + // and SQLRowCount is not > 0. Normally this error is handled in core_sqlsrv_fetch(), + // but if the user calls nextRowset() before fetch() the error is never shown + // so we handle it here. + bool has_result = core_sqlsrv_has_any_result( stmt ); + + if(!driver_stmt->fetch_called){ + CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + } + close_active_stream( stmt TSRMLS_CC ); //Clear column sql types and sql display sizes. diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 07aea0d8c..a548bfc6d 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -561,18 +561,6 @@ PHP_FUNCTION( sqlsrv_next_result ) try { - // Make sure that the result set is not null. Null means SQLNumResultCols returns 0 - // and SQLRowCount is not > 0. Normally this error is handled in core_sqlsrv_fetch(), - // but if the user calls sqlsrv_next_result() before fetch() the error is never shown - // so we handle it here. - bool has_result = core_sqlsrv_has_any_result( stmt ); - - if(!stmt->fetch_called){ - CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { - throw core::CoreException(); - } - } - core_sqlsrv_next_result( stmt TSRMLS_CC, true ); if( stmt->past_next_result_end ) { From 66a12d33303cee7413c8272dbe59538e60c79edc Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Tue, 26 Sep 2017 18:38:48 -0700 Subject: [PATCH 10/27] Fixed errant variable name --- 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 7648ea32b..fdfcf809b 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -1064,7 +1064,7 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC, _In_ bool fin // so we handle it here. bool has_result = core_sqlsrv_has_any_result( stmt ); - if(!driver_stmt->fetch_called){ + if(!stmt->fetch_called){ CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { throw core::CoreException(); } From a749010a6764536776dd03abef9dcf4404933518 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Wed, 27 Sep 2017 17:01:20 -0700 Subject: [PATCH 11/27] Moved errors back to pdo_stmt and stmt.cpp --- source/pdo_sqlsrv/pdo_stmt.cpp | 24 ++++++++++++++++++++++++ source/shared/core_stmt.cpp | 12 ------------ source/sqlsrv/stmt.cpp | 19 +++++++++++++++++++ 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 78828a02e..7fac62592 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,6 +1067,30 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); + // + CHECK_CUSTOM_ERROR( stmt->past_next_result_end, stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { + throw core::CoreException(); + } + + // Make sure that we haven't gone past the end of the result set, then make sure that + // the result set is not null. Null means SQLNumResultCols returns 0 and SQLRowCount + // is not > 0. Normally the latter error is handled in core_sqlsrv_fetch(), but if the + // user calls nextRowset() before fetch() the error is never shown so we handle it here. + // In that case, however, core_sqlsrv_has_any_result would return false if we are at + // the end of a non-null result set, so we check for that error first to make sure the + // user gets the correct error message. + CHECK_CUSTOM_ERROR( stmt->past_next_result_end, driver_stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { + throw core::CoreException(); + } + + bool has_result = core_sqlsrv_has_any_result( driver_stmt ); + + if(!driver_stmt->fetch_called){ + CHECK_CUSTOM_ERROR( !has_result, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + } + core_sqlsrv_next_result( static_cast( stmt->driver_data ) TSRMLS_CC ); // clear the current meta data since the new result will generate new meta data diff --git a/source/shared/core_stmt.cpp b/source/shared/core_stmt.cpp index fdfcf809b..29255029b 100644 --- a/source/shared/core_stmt.cpp +++ b/source/shared/core_stmt.cpp @@ -1058,18 +1058,6 @@ void core_sqlsrv_next_result( _Inout_ sqlsrv_stmt* stmt TSRMLS_DC, _In_ bool fin throw core::CoreException(); } - // Make sure that the result set is not null. Null means SQLNumResultCols returns 0 - // and SQLRowCount is not > 0. Normally this error is handled in core_sqlsrv_fetch(), - // but if the user calls nextRowset() before fetch() the error is never shown - // so we handle it here. - bool has_result = core_sqlsrv_has_any_result( stmt ); - - if(!stmt->fetch_called){ - CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { - throw core::CoreException(); - } - } - close_active_stream( stmt TSRMLS_CC ); //Clear column sql types and sql display sizes. diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index a548bfc6d..4404bf41c 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -561,6 +561,25 @@ PHP_FUNCTION( sqlsrv_next_result ) try { + // Make sure that we haven't gone past the end of the result set, then make sure that + // the result set is not null. Null means SQLNumResultCols returns 0 and SQLRowCount + // is not > 0. Normally the latter error is handled in core_sqlsrv_fetch(), but if the + // user calls sqlsrv_next_result() before fetch() the error is never shown so we handle it here. + // In that case, however, core_sqlsrv_has_any_result would return false if we are at + // the end of a non-null result set, so we check for that error first to make sure the + // user gets the correct error message. + CHECK_CUSTOM_ERROR( stmt->past_next_result_end, stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { + throw core::CoreException(); + } + + bool has_result = core_sqlsrv_has_any_result( driver_stmt ); + + if(!driver_stmt->fetch_called){ + CHECK_CUSTOM_ERROR( !has_result, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { + throw core::CoreException(); + } + } + core_sqlsrv_next_result( stmt TSRMLS_CC, true ); if( stmt->past_next_result_end ) { From d039cf66a7244476145186d2432c3cafe2f5acc8 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Wed, 27 Sep 2017 17:18:38 -0700 Subject: [PATCH 12/27] Fixed typos --- source/sqlsrv/stmt.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 4404bf41c..70f93e2c5 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -572,10 +572,10 @@ PHP_FUNCTION( sqlsrv_next_result ) throw core::CoreException(); } - bool has_result = core_sqlsrv_has_any_result( driver_stmt ); + bool has_result = core_sqlsrv_has_any_result( stmt ); - if(!driver_stmt->fetch_called){ - CHECK_CUSTOM_ERROR( !has_result, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { + if(!stmt->fetch_called){ + CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { throw core::CoreException(); } } From 7713b70d1fe4d89af8dc880647b3c689e570e6de Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Wed, 27 Sep 2017 17:50:20 -0700 Subject: [PATCH 13/27] Fixed typos again --- 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 7fac62592..e75d00607 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1079,7 +1079,7 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) // In that case, however, core_sqlsrv_has_any_result would return false if we are at // the end of a non-null result set, so we check for that error first to make sure the // user gets the correct error message. - CHECK_CUSTOM_ERROR( stmt->past_next_result_end, driver_stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { + CHECK_CUSTOM_ERROR( driver_stmt->past_next_result_end, driver_stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { throw core::CoreException(); } From d56446c41c581f2632a816a004f73fe70ebe7aa2 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Wed, 27 Sep 2017 18:08:24 -0700 Subject: [PATCH 14/27] More tiny fixes --- source/pdo_sqlsrv/pdo_stmt.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index e75d00607..03805f64c 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,11 +1067,6 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); - // - CHECK_CUSTOM_ERROR( stmt->past_next_result_end, stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { - throw core::CoreException(); - } - // Make sure that we haven't gone past the end of the result set, then make sure that // the result set is not null. Null means SQLNumResultCols returns 0 and SQLRowCount // is not > 0. Normally the latter error is handled in core_sqlsrv_fetch(), but if the From 3acab1ae0c0e69cd966cc228e53ea0a34976a6d7 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 28 Sep 2017 14:16:50 -0700 Subject: [PATCH 15/27] Fixed tests and added code comments --- source/pdo_sqlsrv/pdo_stmt.cpp | 19 +- source/sqlsrv/stmt.cpp | 18 +- .../sqlsrv/sqlsrv_empty_result_error.phpt | 13 +- .../sqlsrv/test_warning_errors2.phpt | 327 +++++++++--------- 4 files changed, 202 insertions(+), 175 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index 03805f64c..e6fe5c440 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1067,17 +1067,22 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); - // Make sure that we haven't gone past the end of the result set, then make sure that - // the result set is not null. Null means SQLNumResultCols returns 0 and SQLRowCount - // is not > 0. Normally the latter error is handled in core_sqlsrv_fetch(), but if the - // user calls nextRowset() before fetch() the error is never shown so we handle it here. - // In that case, however, core_sqlsrv_has_any_result would return false if we are at - // the end of a non-null result set, so we check for that error first to make sure the - // user gets the correct error message. + // Return the correct error in case the user calls nextRowset() on a null result set. + // Null means that SQLNumResultCols() returns 0 and SQLRowCount is not return > 0. But first + // check that the statement has been executed and that we are not past the end of a non-null + // result set to make sure the user gets the correct error message. These checks are also + // done in core_sqlsrv_next_result(), but we cannot check for null results there because that + // function can be called without calling this one, and SQLSRV_ERROR_NO_FIELDS can then + // be triggered incorrectly. + CHECK_CUSTOM_ERROR( !driver_stmt->executed, driver_stmt, SQLSRV_ERROR_STATEMENT_NOT_EXECUTED ) { + throw core::CoreException(); + } + CHECK_CUSTOM_ERROR( driver_stmt->past_next_result_end, driver_stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { throw core::CoreException(); } + // Now make sure the result set is not null. bool has_result = core_sqlsrv_has_any_result( driver_stmt ); if(!driver_stmt->fetch_called){ diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 70f93e2c5..33acee7a3 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -561,13 +561,17 @@ PHP_FUNCTION( sqlsrv_next_result ) try { - // Make sure that we haven't gone past the end of the result set, then make sure that - // the result set is not null. Null means SQLNumResultCols returns 0 and SQLRowCount - // is not > 0. Normally the latter error is handled in core_sqlsrv_fetch(), but if the - // user calls sqlsrv_next_result() before fetch() the error is never shown so we handle it here. - // In that case, however, core_sqlsrv_has_any_result would return false if we are at - // the end of a non-null result set, so we check for that error first to make sure the - // user gets the correct error message. + // Return the correct error in case the user calls sqlsrv_next_result() on a null result set. + // Null means that SQLNumResultCols() returns 0 and SQLRowCount is not return > 0. But first + // check that the statement has been executed and that we are not past the end of a non-null + // result set to make sure the user gets the correct error message. These checks are also + // done in core_sqlsrv_next_result(), but we cannot check for null results there because that + // function can be called without calling this one, and SQLSRV_ERROR_NO_FIELDS can then + // be triggered incorrectly. + CHECK_CUSTOM_ERROR( !stmt->executed, stmt, SQLSRV_ERROR_STATEMENT_NOT_EXECUTED ) { + throw core::CoreException(); + } + CHECK_CUSTOM_ERROR( stmt->past_next_result_end, stmt, SQLSRV_ERROR_NEXT_RESULT_PAST_END ) { throw core::CoreException(); } diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index f073d4e51..af0dd2f8e 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -68,13 +68,22 @@ Array Array ( [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -26 + [code] => -26 + [2] => There are no more results returned by the query. + [message] => There are no more results returned by the query. + ) + [1] => Array ( [0] => HY010 [SQLSTATE] => HY010 [1] => 0 [code] => 0 - [2] => [unixODBC][Driver Manager]Function sequence error - [message] => [unixODBC][Driver Manager]Function sequence error + [2] => [Microsoft][ODBC Driver Manager] Function sequence error + [message] => [Microsoft][ODBC Driver Manager] Function sequence error ) ) diff --git a/test/functional/sqlsrv/test_warning_errors2.phpt b/test/functional/sqlsrv/test_warning_errors2.phpt index 3372ca895..8ee123526 100644 --- a/test/functional/sqlsrv/test_warning_errors2.phpt +++ b/test/functional/sqlsrv/test_warning_errors2.phpt @@ -1,159 +1,168 @@ ---TEST-- -warnings as errors ---SKIPIF-- - ---FILE-- - ---EXPECT-- -Array -( - [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. - ) - -) -Array -( - [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. - ) - -) -Array -( - [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. - ) - -) -Array -( - [0] => Array - ( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. - ) - -) -Array -( - [0] => IMSSP - [SQLSTATE] => IMSSP - [1] => -11 - [code] => -11 - [2] => The statement must be executed before results can be retrieved. - [message] => The statement must be executed before results can be retrieved. -) -Test successful +--TEST-- +warnings as errors +--SKIPIF-- + +--FILE-- + +--EXPECT-- +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. + ) + +) +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. + ) + +) +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. + ) + +) +Array +( + [0] => Array + ( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. + ) + [1] => Array + ( + [0] => HY010 + [SQLSTATE] => HY010 + [1] => 0 + [code] => 0 + [2] => [unixODBC][Driver Manager]Function sequence error + [message] => [unixODBC][Driver Manager]Function sequence error + ) + +) +Array +( + [0] => IMSSP + [SQLSTATE] => IMSSP + [1] => -11 + [code] => -11 + [2] => The statement must be executed before results can be retrieved. + [message] => The statement must be executed before results can be retrieved. +) +Test successful From 257b1cd6717355e97c3f474b83efa47a6ae938c6 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 28 Sep 2017 15:00:14 -0700 Subject: [PATCH 16/27] Fixed test outputs --- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 1 + test/functional/sqlsrv/test_warning_errors2.phpt | 9 --------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index af0dd2f8e..adf327bb8 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -76,6 +76,7 @@ Array [2] => There are no more results returned by the query. [message] => There are no more results returned by the query. ) + [1] => Array ( [0] => HY010 diff --git a/test/functional/sqlsrv/test_warning_errors2.phpt b/test/functional/sqlsrv/test_warning_errors2.phpt index 8ee123526..4145565ac 100644 --- a/test/functional/sqlsrv/test_warning_errors2.phpt +++ b/test/functional/sqlsrv/test_warning_errors2.phpt @@ -145,15 +145,6 @@ Array [2] => The statement must be executed before results can be retrieved. [message] => The statement must be executed before results can be retrieved. ) - [1] => Array - ( - [0] => HY010 - [SQLSTATE] => HY010 - [1] => 0 - [code] => 0 - [2] => [unixODBC][Driver Manager]Function sequence error - [message] => [unixODBC][Driver Manager]Function sequence error - ) ) Array From 7315d4426aa279c939153042ffe4693547b90452 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Thu, 28 Sep 2017 15:19:38 -0700 Subject: [PATCH 17/27] Fixed test output --- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 6 +++--- 1 file 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 adf327bb8..bf43e3b68 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -58,7 +58,7 @@ $stmt = sqlsrv_query($conn, "DROP TABLE TestEmptySetTable"); $stmt = sqlsrv_query($conn, "DROP PROCEDURE TestEmptySetProc"); sqlsrv_close($conn); ?> ---EXPECT-- +--EXPECTF-- Return a nonempty result set: Array ( @@ -83,8 +83,8 @@ Array [SQLSTATE] => HY010 [1] => 0 [code] => 0 - [2] => [Microsoft][ODBC Driver Manager] Function sequence error - [message] => [Microsoft][ODBC Driver Manager] Function sequence error + [2] => [%s][ODBC Driver Manager] Function sequence error + [message] => [%s][ODBC Driver Manager] Function sequence error ) ) From 494527d465fa7d1497448d24963e2b128bf9efd1 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 29 Sep 2017 14:30:29 -0700 Subject: [PATCH 18/27] Tweaked tests, added comments per code review comments --- source/pdo_sqlsrv/pdo_stmt.cpp | 3 +++ source/sqlsrv/stmt.cpp | 3 +++ test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt | 7 ++++--- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 6 ++++-- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index e6fe5c440..fe2ff5fae 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1085,6 +1085,9 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) // Now make sure the result set is not null. bool has_result = core_sqlsrv_has_any_result( driver_stmt ); + // Note that if fetch_called is false but has_result is true (i.e. the user is calling + // nextRowset() on a non-null result set before calling fetch()), it is handled + // in core_sqlsrv_next_result() below. if(!driver_stmt->fetch_called){ CHECK_CUSTOM_ERROR( !has_result, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { throw core::CoreException(); diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 33acee7a3..8f5abb6c9 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -578,6 +578,9 @@ PHP_FUNCTION( sqlsrv_next_result ) bool has_result = core_sqlsrv_has_any_result( stmt ); + // Note that if fetch_called is false but has_result is true (i.e. the user is calling + // sqlsrv_next_result() on a non-null result set before calling fetch()), it is handled + // in core_sqlsrv_next_result() below. if(!stmt->fetch_called){ CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { throw core::CoreException(); diff --git a/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt index 660ff5062..dda981561 100644 --- a/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt +++ b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt @@ -9,11 +9,12 @@ require_once("MsSetup.inc"); $conn = new PDO( "sqlsrv:Server = $server; Database = $databaseName; ", $uid, $pwd ); $conn->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION ); -$stmt = $conn->query("IF OBJECT_ID('TestEmptySetTable', 'U') IS NOT NULL DROP TABLE TestEmptySetTable"); +DropTable($conn, 'TestEmptySetTable'); $stmt = $conn->query("CREATE TABLE TestEmptySetTable ([c1] nvarchar(10),[c2] nvarchar(10))"); - $stmt = $conn->query("INSERT INTO TestEmptySetTable (c1, c2) VALUES ('a', 'b')"); -$stmt = $conn->query("IF OBJECT_ID('TestEmptySetProc', 'P') IS NOT NULL DROP PROCEDURE TestEmptySetProc"); + +// Create a procedure that can return a result set or can return nothing +DropProc($conn, 'TestEmptySetProc'); $stmt = $conn->query("CREATE PROCEDURE TestEmptySetProc @a nvarchar(10), @b nvarchar(10) AS SET NOCOUNT ON BEGIN diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index bf43e3b68..ba07801e9 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -8,10 +8,12 @@ require_once("MsSetup.inc"); $conn = sqlsrv_connect($server, array("Database"=>$databaseName, "uid"=>$uid, "pwd"=>$pwd)); -$stmt = sqlsrv_query($conn, "IF OBJECT_ID('TestEmptySetTable', 'U') IS NOT NULL DROP TABLE TestEmptySetTable"); +DropTable($conn, 'TestEmptySetTable'); $stmt = sqlsrv_query($conn, "CREATE TABLE TestEmptySetTable ([c1] nvarchar(10),[c2] nvarchar(10))"); $stmt = sqlsrv_query($conn, "INSERT INTO TestEmptySetTable (c1, c2) VALUES ('a', 'b')"); -$stmt = sqlsrv_query($conn, "IF OBJECT_ID('TestEmptySetProc', 'P') IS NOT NULL DROP PROCEDURE TestEmptySetProc"); + +// Create a procedure that can return a result set or can return nothing +DropProc($conn, 'TestEmptySetProc'); $stmt = sqlsrv_query($conn, "CREATE PROCEDURE TestEmptySetProc @a nvarchar(10), @b nvarchar(10) AS SET NOCOUNT ON BEGIN From 0728a1da5ab8e23ebd2a0fee6f4534bef17cca1a Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 29 Sep 2017 14:52:33 -0700 Subject: [PATCH 19/27] Update pdo_empty_result_error.phpt --- test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt index dda981561..583dc2aed 100644 --- a/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt +++ b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt @@ -5,6 +5,7 @@ Test that calling nextRowset() on an empty result set produces the correct error --FILE-- setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION ); From dc6e2a34fa67d844158cdd9483bef13c7e383916 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 29 Sep 2017 14:52:51 -0700 Subject: [PATCH 20/27] Update sqlsrv_empty_result_error.phpt --- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index ba07801e9..c3085f7c0 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -5,6 +5,7 @@ Test that calling sqlsrv_next_result() on a null result set produces the correct --FILE-- $databaseName, "uid"=>$uid, "pwd"=>$pwd)); From 9e928672d489354cb29dac21db3ee1d75477c68c Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 29 Sep 2017 16:17:39 -0700 Subject: [PATCH 21/27] Update pdo_stmt.cpp --- 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 fe2ff5fae..d0b487d03 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1068,7 +1068,7 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) SQLSRV_ASSERT( driver_stmt != NULL, "pdo_sqlsrv_stmt_next_rowset: driver_data object was null" ); // Return the correct error in case the user calls nextRowset() on a null result set. - // Null means that SQLNumResultCols() returns 0 and SQLRowCount is not return > 0. But first + // Null means that SQLNumResultCols() returns 0 and SQLRowCount does not return > 0. But first // check that the statement has been executed and that we are not past the end of a non-null // result set to make sure the user gets the correct error message. These checks are also // done in core_sqlsrv_next_result(), but we cannot check for null results there because that From 4674f58a15007e6527fb4ff3deff7e4252fd4b16 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 29 Sep 2017 16:18:27 -0700 Subject: [PATCH 22/27] Update stmt.cpp --- source/sqlsrv/stmt.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 8f5abb6c9..076dba1a6 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -562,7 +562,7 @@ PHP_FUNCTION( sqlsrv_next_result ) try { // Return the correct error in case the user calls sqlsrv_next_result() on a null result set. - // Null means that SQLNumResultCols() returns 0 and SQLRowCount is not return > 0. But first + // Null means that SQLNumResultCols() returns 0 and SQLRowCount does not return > 0. But first // check that the statement has been executed and that we are not past the end of a non-null // result set to make sure the user gets the correct error message. These checks are also // done in core_sqlsrv_next_result(), but we cannot check for null results there because that From 4151f8f5d1d4c2bb8f31d561a9a96ae37c895d5f Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Fri, 29 Sep 2017 20:47:24 -0700 Subject: [PATCH 23/27] Minor fixes --- source/pdo_sqlsrv/pdo_stmt.cpp | 2 +- source/sqlsrv/stmt.cpp | 2 +- test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt | 2 ++ test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 3 +++ 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/source/pdo_sqlsrv/pdo_stmt.cpp b/source/pdo_sqlsrv/pdo_stmt.cpp index fe2ff5fae..57ebd9d5a 100644 --- a/source/pdo_sqlsrv/pdo_stmt.cpp +++ b/source/pdo_sqlsrv/pdo_stmt.cpp @@ -1088,7 +1088,7 @@ int pdo_sqlsrv_stmt_next_rowset( _Inout_ pdo_stmt_t *stmt TSRMLS_DC ) // Note that if fetch_called is false but has_result is true (i.e. the user is calling // nextRowset() on a non-null result set before calling fetch()), it is handled // in core_sqlsrv_next_result() below. - if(!driver_stmt->fetch_called){ + if( !driver_stmt->fetch_called ) { CHECK_CUSTOM_ERROR( !has_result, driver_stmt, SQLSRV_ERROR_NO_FIELDS ) { throw core::CoreException(); } diff --git a/source/sqlsrv/stmt.cpp b/source/sqlsrv/stmt.cpp index 8f5abb6c9..b9c3bbfb7 100644 --- a/source/sqlsrv/stmt.cpp +++ b/source/sqlsrv/stmt.cpp @@ -581,7 +581,7 @@ PHP_FUNCTION( sqlsrv_next_result ) // Note that if fetch_called is false but has_result is true (i.e. the user is calling // sqlsrv_next_result() on a non-null result set before calling fetch()), it is handled // in core_sqlsrv_next_result() below. - if(!stmt->fetch_called){ + if( !stmt->fetch_called ) { CHECK_CUSTOM_ERROR( !has_result, stmt, SQLSRV_ERROR_NO_FIELDS ) { throw core::CoreException(); } diff --git a/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt index dda981561..ae3bf414a 100644 --- a/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt +++ b/test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt @@ -1,4 +1,6 @@ --TEST-- +Error messages from null result sets +--DESCRIPTION-- Test that calling nextRowset() on an empty result set produces the correct error message. Fix for Github 507. --SKIPIF-- diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index ba07801e9..8c2210e28 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -1,4 +1,6 @@ --TEST-- +Error messages from null result sets +--DESCRIPTION-- Test that calling sqlsrv_next_result() on a null result set produces the correct error message. Fix for Github 507. --SKIPIF-- @@ -58,6 +60,7 @@ print_r(sqlsrv_errors()); $stmt = sqlsrv_query($conn, "DROP TABLE TestEmptySetTable"); $stmt = sqlsrv_query($conn, "DROP PROCEDURE TestEmptySetProc"); +sqlsrv_free_stmt($stmt); sqlsrv_close($conn); ?> --EXPECTF-- From cc0d71d1633cc3911fb83e298e4d53d374cf5289 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Mon, 2 Oct 2017 14:10:12 -0700 Subject: [PATCH 24/27] Fixed EXPECTF test output --- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index f5d2409c5..0b60ad933 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -89,8 +89,8 @@ Array [SQLSTATE] => HY010 [1] => 0 [code] => 0 - [2] => [%s][ODBC Driver Manager] Function sequence error - [message] => [%s][ODBC Driver Manager] Function sequence error + [2] => [%rMicrosoft|unixODBC%r][%rODBC D|D%rriver Manager] Function sequence error + [message] => [%rMicrosoft|unixODBC%r][%rODBC D|D%rriver Manager] Function sequence error ) ) From 6aa062f7d6003823902baeb7ec51cd73f6b466d1 Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Mon, 2 Oct 2017 15:28:51 -0700 Subject: [PATCH 25/27] Changed mssql-server-linux tag --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index dfe1b0f22..8f89cccc3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,7 +20,7 @@ env: - TEST_PHP_SQL_PWD=Password123 before_install: - - docker pull microsoft/mssql-server-linux + - docker pull microsoft/mssql-server-linux:2017-latest install: - docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=Password123' -p 1433:1433 --name=$TEST_PHP_SQL_SERVER -d microsoft/mssql-server-linux From 4349a3c029808a5efdf8ae27db4860baba9b33cd Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Mon, 2 Oct 2017 15:36:03 -0700 Subject: [PATCH 26/27] Changed mssql-server-linux tag --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8f89cccc3..5a5334eac 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,7 +23,7 @@ before_install: - docker pull microsoft/mssql-server-linux:2017-latest install: - - docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=Password123' -p 1433:1433 --name=$TEST_PHP_SQL_SERVER -d microsoft/mssql-server-linux + - docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=Password123' -p 1433:1433 --name=$TEST_PHP_SQL_SERVER -d microsoft/mssql-server-linux:2017-latest - docker build --build-arg PHPSQLDIR=$PHPSQLDIR -t msphpsql-dev -f Dockerfile-msphpsql . before_script: From 70b20a2bb968b50770c8e647fae2ca93bfae3bba Mon Sep 17 00:00:00 2001 From: David Puglielli Date: Mon, 2 Oct 2017 16:19:23 -0700 Subject: [PATCH 27/27] Fixed regex output --- test/functional/sqlsrv/sqlsrv_empty_result_error.phpt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt index 0b60ad933..200ec1ac7 100644 --- a/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt +++ b/test/functional/sqlsrv/sqlsrv_empty_result_error.phpt @@ -89,8 +89,8 @@ Array [SQLSTATE] => HY010 [1] => 0 [code] => 0 - [2] => [%rMicrosoft|unixODBC%r][%rODBC D|D%rriver Manager] Function sequence error - [message] => [%rMicrosoft|unixODBC%r][%rODBC D|D%rriver Manager] Function sequence error + [2] => [%rMicrosoft|unixODBC%r][%rODBC D|D%rriver Manager]%r[ ]{0,1}%rFunction sequence error + [message] => [%rMicrosoft|unixODBC%r][%rODBC D|D%rriver Manager]%r[ ]{0,1}%rFunction sequence error ) )