Added empty result set check to nextRowset()

.travis.yml

@@ -20,10 +20,10 @@ 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
+  - 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:

source/pdo_sqlsrv/pdo_stmt.cpp

@@ -1067,6 +1067,33 @@ 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 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
+        // 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 );
+
+        // 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();
+            }
+        }
+
         core_sqlsrv_next_result( static_cast<sqlsrv_stmt*>( stmt->driver_data ) TSRMLS_CC );
 
         // clear the current meta data since the new result will generate new meta data

source/shared/core_stmt.cpp

@@ -1057,7 +1057,7 @@ 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();
         }
-
+        
         close_active_stream( stmt TSRMLS_CC );
 
         //Clear column sql types and sql display sizes.

source/sqlsrv/stmt.cpp

@@ -561,6 +561,32 @@ 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 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
+        // 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();
+        }
+
+        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();
+            }
+        }
+
         core_sqlsrv_next_result( stmt TSRMLS_CC, true );
 
         if( stmt->past_next_result_end ) {

test/functional/pdo_sqlsrv/pdo_empty_result_error.phpt

@@ -0,0 +1,100 @@
+--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--
+<?php require('skipif.inc'); ?>
+--FILE--
+<?php
+require_once("MsSetup.inc");
+require_once("MsCommon.inc");
+
+$conn = new PDO( "sqlsrv:Server = $server; Database = $databaseName; ", $uid, $pwd );
+$conn->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
+
+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')");
+
+// 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
+                          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.

test/functional/sqlsrv/sqlsrv_empty_result_error.phpt

@@ -0,0 +1,124 @@
+--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--
+<?php require('skipif.inc'); ?>
+--FILE--
+<?php
+require_once("MsSetup.inc");
+require_once("MsCommon.inc");
+
+$conn = sqlsrv_connect($server, array("Database"=>$databaseName, "uid"=>$uid, "pwd"=>$pwd));
+
+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')");
+
+// 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
+                                 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_stmt($stmt);
+sqlsrv_close($conn);
+?>
+--EXPECTF--
+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] => [%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
+        )
+
+)
+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.
+        )
+
+)