Merge branch 'w04_MDL-37734_m23_marshack' of git://github.com/skodak/…

…moodle into MOODLE_23_STABLE
moodle · Jan 30, 2013 · 2cc9648 · 2cc9648
2 parents 06ffb65 + 9834ef1
commit 2cc9648
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 7 deletions.
diff --git a/lib/dml/sqlsrv_native_moodle_database.php b/lib/dml/sqlsrv_native_moodle_database.php
@@ -41,6 +41,8 @@ class sqlsrv_native_moodle_database extends moodle_database {
     protected $last_error_reporting; // To handle SQL*Server-Native driver default verbosity
     protected $temptables; // Control existing temptables (sqlsrv_moodle_temptables object)
     protected $collation;  // current DB collation cache
+    /** @var array list of open recordsets */
+    protected $recordsets = array();
 
     /**
      * Constructor - instantiates the database, specifying if it's external (connect to other systems) or no (Moodle DB)
@@ -789,7 +791,20 @@ public function get_recordset_sql($sql, array $params = null, $limitfrom = 0, $l
      * @return sqlsrv_native_moodle_recordset
      */
     protected function create_recordset($result) {
-        return new sqlsrv_native_moodle_recordset($result);
+        $rs = new sqlsrv_native_moodle_recordset($result, $this);
+        $this->recordsets[] = $rs;
+        return $rs;
+    }
+
+    /**
+     * Do not use outside of recordset class.
+     * @internal
+     * @param sqlsrv_native_moodle_recordset $rs
+     */
+    public function recordset_closed(sqlsrv_native_moodle_recordset $rs) {
+        if ($key = array_search($rs, $this->recordsets, true)) {
+            unset($this->recordsets[$key]);
+        }
     }
 
     /**
@@ -1363,6 +1378,12 @@ public function release_session_lock($rowid) {
      * @return void
      */
     protected function begin_transaction() {
+        // Recordsets do not work well with transactions in SQL Server,
+        // let's prefetch the recordsets to memory to work around these problems.
+        foreach ($this->recordsets as $rs) {
+            $rs->transaction_starts();
+        }
+
         $this->query_start('native sqlsrv_begin_transaction', NULL, SQL_QUERY_AUX);
         $result = sqlsrv_begin_transaction($this->sqlsrv);
         $this->query_end($result);

diff --git a/lib/dml/sqlsrv_native_moodle_recordset.php b/lib/dml/sqlsrv_native_moodle_recordset.php
@@ -31,20 +31,68 @@ class sqlsrv_native_moodle_recordset extends moodle_recordset {
     protected $rsrc;
     protected $current;
 
-    public function __construct($rsrc) {
+    /** @var array recordset buffer */
-        $this->rsrc  = $rsrc;
+    protected $buffer = null;
+
+    /** @var sqlsrv_native_moodle_database */
+    protected $db;
+
+    public function __construct($rsrc, sqlsrv_native_moodle_database $db) {
+        $this->rsrc    = $rsrc;
         $this->current = $this->fetch_next();
+        $this->db      = $db;
+    }
+
+    /**
+     * Inform existing open recordsets that transaction
+     * is starting, this works around MARS problem described
+     * in MDL-37734.
+     */
+    public function transaction_starts() {
+        if ($this->buffer !== null) {
+            $this->unregister();
+            return;
+        }
+        if (!$this->rsrc) {
+            $this->unregister();
+            return;
+        }
+        // This might eat memory pretty quickly...
+        raise_memory_limit('2G');
+        $this->buffer = array();
+
+        while($next = $this->fetch_next()) {
+            $this->buffer[] = $next;
+        }
+    }
+
+    /**
+     * Unregister recordset from the global list of open recordsets.
+     */
+    private function unregister() {
+        if ($this->db) {
+            $this->db->recordset_closed($this);
+            $this->db = null;
+        }
     }
 
     public function __destruct() {
         $this->close();
     }
 
     private function fetch_next() {
-        if ($row = sqlsrv_fetch_array($this->rsrc, SQLSRV_FETCH_ASSOC)) {
+        if (!$this->rsrc) {
-            unset($row['sqlsrvrownumber']);
+            return false;
-            $row = array_change_key_case($row, CASE_LOWER);
         }
+        if (!$row = sqlsrv_fetch_array($this->rsrc, SQLSRV_FETCH_ASSOC)) {
+            sqlsrv_free_stmt($this->rsrc);
+            $this->rsrc = null;
+            $this->unregister();
+            return false;
+        }
+
+        unset($row['sqlsrvrownumber']);
+        $row = array_change_key_case($row, CASE_LOWER);
         return $row;
     }
 
@@ -62,7 +110,11 @@ public function key() {
     }
 
     public function next() {
-        $this->current = $this->fetch_next();
+        if ($this->buffer === null) {
+            $this->current = $this->fetch_next();
+        } else {
+            $this->current = array_shift($this->buffer);
+        }
     }
 
     public function valid() {
@@ -75,5 +127,7 @@ public function close() {
             $this->rsrc  = null;
         }
         $this->current = null;
+        $this->buffer  = null;
+        $this->unregister();
     }
 }
diff --git a/lib/dml/tests/dml_test.php b/lib/dml/tests/dml_test.php
@@ -4314,6 +4314,35 @@ function test_nested_transactions() {
         }
         $rs1->close();
         $this->assertEquals(3, $i);
+
+        // Test nested recordsets isolation without transaction.
+        $DB->delete_records($tablename);
+        $DB->insert_record($tablename, array('course'=>1));
+        $DB->insert_record($tablename, array('course'=>2));
+        $DB->insert_record($tablename, array('course'=>3));
+
+        $DB->delete_records($tablename2);
+        $DB->insert_record($tablename2, array('course'=>5));
+        $DB->insert_record($tablename2, array('course'=>6));
+        $DB->insert_record($tablename2, array('course'=>7));
+        $DB->insert_record($tablename2, array('course'=>8));
+
+        $rs1 = $DB->get_recordset($tablename);
+        $i = 0;
+        foreach ($rs1 as $record1) {
+            $i++;
+            $rs2 = $DB->get_recordset($tablename2);
+            $j = 0;
+            foreach ($rs2 as $record2) {
+                $DB->set_field($tablename, 'course', $record1->course+1, array('id'=>$record1->id));
+                $DB->set_field($tablename2, 'course', $record2->course+1, array('id'=>$record2->id));
+                $j++;
+            }
+            $rs2->close();
+            $this->assertEquals(4, $j);
+        }
+        $rs1->close();
+        $this->assertEquals(3, $i);
     }
 
     function test_transactions_forbidden() {