Dropped the use of LOCK TIMEOUT (#1165)

Author: yitam

source/pdo_sqlsrv/pdo_stmt.cpp

@@ -1523,12 +1523,3 @@ sqlsrv_phptype pdo_sqlsrv_stmt::sql_type_to_php_type( _In_ SQLINTEGER sql_type,
 
     return sqlsrv_phptype;
 }
-
-void pdo_sqlsrv_stmt::set_query_timeout()
-{
-    if (query_timeout == QUERY_TIMEOUT_INVALID || query_timeout < 0) {
-        return;
-    }
-
-    core::SQLSetStmtAttr(this, SQL_ATTR_QUERY_TIMEOUT, reinterpret_cast<SQLPOINTER>((SQLLEN)query_timeout), SQL_IS_UINTEGER);
-}

source/pdo_sqlsrv/php_pdo_sqlsrv_int.h

@@ -260,9 +260,6 @@ struct pdo_sqlsrv_stmt : public sqlsrv_stmt {
     // for PDO, everything is a string, so we return SQLSRV_PHPTYPE_STRING for all SQL types
     virtual sqlsrv_phptype sql_type_to_php_type( _In_ SQLINTEGER sql_type, _In_ SQLUINTEGER size, _In_ bool prefer_string_to_stream );
 
-    // driver specific way to set query timeout
-    virtual void set_query_timeout();
-
     bool direct_query;                        // flag set if the query should be executed directly or prepared
     const char* direct_query_subst_string;    // if the query is direct, hold the substitution string if using named parameters
     size_t direct_query_subst_string_len;        // length of query string used for direct queries

source/shared/core_sqlsrv.h

@@ -1520,6 +1520,8 @@ struct sqlsrv_stmt : public sqlsrv_context {
 
     // free sensitivity classification metadata
     void clean_up_sensitivity_metadata();
+    // set query timeout
+    void set_query_timeout();
 
     sqlsrv_conn*   conn;                  // Connection that created this statement
 
@@ -1571,8 +1573,6 @@ struct sqlsrv_stmt : public sqlsrv_context {
     // driver specific conversion rules from a SQL Server/ODBC type to one of the SQLSRV_PHPTYPE_* constants
     virtual sqlsrv_phptype sql_type_to_php_type( _In_ SQLINTEGER sql_type, _In_ SQLUINTEGER size, _In_ bool prefer_string_to_stream ) = 0;
 
-    // driver specific way to set query timeout
-    virtual void set_query_timeout() = 0;
 };
 
 // *** field metadata struct ***

source/shared/core_stmt.cpp

@@ -275,6 +275,15 @@ void sqlsrv_stmt::clean_up_sensitivity_metadata()
     }
 }
 
+void sqlsrv_stmt::set_query_timeout()
+{
+    if (query_timeout == QUERY_TIMEOUT_INVALID || query_timeout < 0) {
+        return;
+    }
+
+    core::SQLSetStmtAttr(this, SQL_ATTR_QUERY_TIMEOUT, reinterpret_cast<SQLPOINTER>((SQLLEN)query_timeout), SQL_IS_UINTEGER);
+}
+
 // core_sqlsrv_create_stmt
 // Common code to allocate a statement from either driver.  Returns a valid driver statement object or
 // throws an exception if an error occurs.

source/sqlsrv/php_sqlsrv_int.h

@@ -130,9 +130,6 @@ struct ss_sqlsrv_stmt : public sqlsrv_stmt {
     // driver specific conversion rules from a SQL Server/ODBC type to one of the SQLSRV_PHPTYPE_* constants
     sqlsrv_phptype sql_type_to_php_type( _In_ SQLINTEGER sql_type, _In_ SQLUINTEGER size, _In_ bool prefer_string_to_stream );
 
-    // driver specific way to set query timeout
-    virtual void set_query_timeout();
-
     bool prepared;                               // whether the statement has been prepared yet (used for error messages)
     zend_ulong conn_index;                       // index into the connection hash that contains this statement structure
     zval* params_z;                              // hold parameters passed to sqlsrv_prepare but not used until sqlsrv_execute

source/sqlsrv/stmt.cpp

@@ -261,29 +261,6 @@ sqlsrv_phptype ss_sqlsrv_stmt::sql_type_to_php_type( _In_ SQLINTEGER sql_type, _
     return ss_phptype;
 }
 
-void ss_sqlsrv_stmt::set_query_timeout()
-{
-    if (query_timeout == QUERY_TIMEOUT_INVALID || query_timeout < 0) {
-        return;
-    }
-
-    // set the statement attribute
-    core::SQLSetStmtAttr(this, SQL_ATTR_QUERY_TIMEOUT, reinterpret_cast<SQLPOINTER>( (SQLLEN)query_timeout ), SQL_IS_UINTEGER );
-
-    // a query timeout of 0 indicates "no timeout", which means that lock_timeout should also be set to "no timeout" which
-    // is represented by -1.
-    int lock_timeout = (( query_timeout == 0 ) ? -1 : query_timeout * 1000 /*convert to milliseconds*/ );
-
-    // set the LOCK_TIMEOUT on the server.
-    char lock_timeout_sql[32] = {'\0'};
-
-    int written = snprintf( lock_timeout_sql, sizeof( lock_timeout_sql ), "SET LOCK_TIMEOUT %d", lock_timeout );
-    SQLSRV_ASSERT( (written != -1 && written != sizeof( lock_timeout_sql )),
-                  "stmt_option_query_timeout: snprintf failed. Shouldn't ever fail." );
-
-    core::SQLExecDirect(this, lock_timeout_sql );
-}
-
 // statement specific parameter proccessing.  Uses the generic function specialised to return a statement
 // resource.
 #define PROCESS_PARAMS( rsrc, param_spec, calling_func, param_count, ... )                                                        \

test/functional/pdo_sqlsrv/pdo_1100_query_timeout_disconnect.phpt

@@ -0,0 +1,72 @@
+--TEST--
+GitHub issue 1100 - PDO::SQLSRV_ATTR_QUERY_TIMEOUT had no effect when reconnecting
+--DESCRIPTION--
+This test verifies that setting PDO::SQLSRV_ATTR_QUERY_TIMEOUT should work when reconnecting after disconnecting
+--ENV--
+PHPT_EXEC=true
+--SKIPIF--
+<?php require('skipif_mid-refactor.inc'); ?>
+--FILE--
+<?php
+require_once("MsSetup.inc");
+require_once("MsCommon_mid-refactor.inc");
+
+function checkTimeElapsed($t0, $t1, $expectedDelay)
+{
+    $elapsed = $t1 - $t0;
+    $diff = abs($elapsed - $expectedDelay);
+    $leeway = 1.0;
+    $missed = ($diff > $leeway);
+    trace("$elapsed secs elapsed\n");
+
+    if ($missed) {
+        echo "Expected $expectedDelay but $elapsed secs elapsed\n";
+    }
+}
+
+function testTimeout($conn, $timeout)
+{
+    $delay = 5;
+    $query = "WAITFOR DELAY '00:00:$delay'; SELECT 1";
+    $error = '*Query timeout expired';
+
+    $t0 = microtime(true);
+    try {
+        $conn->exec($query);
+        $elapsed = microtime(true) - $t0;
+        echo "Should have failed after $timeout secs but $elapsed secs have elapsed" . PHP_EOL;
+    } catch (PDOException $e) {
+        $t1 = microtime(true);
+        
+        $message = '*Query timeout expired';
+        if (!fnmatch($message, $e->getMessage())) {
+            var_dump($e->getMessage());
+        }
+        checkTimeElapsed($t0, $t1, $timeout);
+    }
+}
+
+try {
+    $keywords = 'MultipleActiveResultSets=false;';
+    $timeout = 1;
+    
+    $options = array(PDO::SQLSRV_ATTR_QUERY_TIMEOUT => $timeout);
+    $conn = connect($keywords, $options);
+
+    testTimeout($conn, $timeout);
+    unset($conn);
+
+    $conn = connect($keywords);
+    $conn->setAttribute(PDO::SQLSRV_ATTR_QUERY_TIMEOUT, $timeout);
+
+    testTimeout($conn, $timeout);
+    unset($conn);
+
+    echo "Done\n";
+} catch (PdoException $e) {
+    echo $e->getMessage() . PHP_EOL;
+}
+
+?>
+--EXPECT--
+Done

test/functional/sqlsrv/test_timeout.phpt

@@ -10,19 +10,23 @@ sqlsrv_configure( 'LogSeverity', SQLSRV_LOG_SEVERITY_ALL );
 
 require( 'MsCommon.inc' );
 
-$throwaway = Connect(array( 'ConnectionPooling' => 1 ));
+// MARS allows applications to have more than one pending request per connection and to have more 
+// than one active default result set per connection, which is not required for this test.
+$options = array('ConnectionPooling' => 1, 'MultipleActiveResultSets' => 0);
+
+$throwaway = Connect();
 if( !$throwaway ) {
     die( print_r( sqlsrv_errors(), true ));
 }
 
 for( $i = 1; $i <= 3; ++$i ) {
 
-    $conn = Connect(array( 'ConnectionPooling' => 1 ));
+    $conn = Connect($options);
     if( !$conn ) {
         die( print_r( sqlsrv_errors(), true ));
     }
 
-    $conn2 = Connect(array( 'ConnectionPooling' => 1 ));
+    $conn2 = Connect($options);
     if( !$conn2 ) {
       die( print_r( sqlsrv_errors(), true ));
     }
@@ -47,7 +51,6 @@ for( $i = 1; $i <= 3; ++$i ) {
       die( print_r( sqlsrv_errors(), true ));
     }
 
-
     $stmt2 = sqlsrv_query( $conn2, "WAITFOR DELAY '00:00:05'; SELECT * FROM [test_query_timeout]", array(null), array( 'QueryTimeout' => 1 ));
     if( $stmt2 === false ) {
         print_r( sqlsrv_errors() );