From 6128bf9d2501eba33fcf769454e3566dbcac7809 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C5=A0koda?= <commits@skodak.org>
Date: Tue, 3 Jul 2012 20:00:21 +0200
Subject: [PATCH] MDL-34159 improve where_clause_list performance

---
 lib/dml/moodle_database.php | 37 ++++++++++++++-----------
 lib/dml/tests/dml_test.php  | 54 +++++++++++++++++++++++++++++++++++--
 2 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/lib/dml/moodle_database.php b/lib/dml/moodle_database.php
index 48a992c73eb..b44b740fbf2 100644
--- a/lib/dml/moodle_database.php
+++ b/lib/dml/moodle_database.php
@@ -574,21 +574,38 @@ abstract class moodle_database {
      * @return array An array containing sql 'where' part and 'params'
      */
     protected function where_clause_list($field, array $values) {
+        if (empty($values)) {
+            return array("1 = 2", array());
+        }
+
+        // Note: Do not use get_in_or_equal() because it can not deal with bools and nulls.
+
         $params = array();
-        $select = array();
+        $select = "";
         $values = (array)$values;
         foreach ($values as $value) {
             if (is_bool($value)) {
                 $value = (int)$value;
             }
             if (is_null($value)) {
-                $select[] = "$field IS NULL";
+                $select = "$field IS NULL";
             } else {
-                $select[] = "$field = ?";
                 $params[] = $value;
             }
         }
-        $select = implode(" OR ", $select);
+        if ($params) {
+            if ($select !== "") {
+                $select = "$select OR ";
+            }
+            $count = count($params);
+            if ($count == 1) {
+                $select = $select."$field = ?";
+            } else {
+                $qs = str_repeat(',?', $count);
+                $qs = ltrim($qs, ',');
+                $select = $select."$field IN ($qs)";
+            }
+        }
         return array($select, $params);
     }
 
@@ -1034,10 +1051,6 @@ abstract class moodle_database {
      */
     public function get_recordset_list($table, $field, array $values, $sort='', $fields='*', $limitfrom=0, $limitnum=0) {
         list($select, $params) = $this->where_clause_list($field, $values);
-        if (empty($select)) {
-            $select = '1 = 2'; // Fake condition, won't return rows ever. MDL-17645
-            $params = array();
-        }
         return $this->get_recordset_select($table, $select, $params, $sort, $fields, $limitfrom, $limitnum);
     }
 
@@ -1132,10 +1145,6 @@ abstract class moodle_database {
      */
     public function get_records_list($table, $field, array $values, $sort='', $fields='*', $limitfrom=0, $limitnum=0) {
         list($select, $params) = $this->where_clause_list($field, $values);
-        if (empty($select)) {
-            // nothing to return
-            return array();
-        }
         return $this->get_records_select($table, $select, $params, $sort, $fields, $limitfrom, $limitnum);
     }
 
@@ -1662,10 +1671,6 @@ abstract class moodle_database {
      */
     public function delete_records_list($table, $field, array $values) {
         list($select, $params) = $this->where_clause_list($field, $values);
-        if (empty($select)) {
-            // nothing to delete
-            return true;
-        }
         return $this->delete_records_select($table, $select, $params);
     }
 
diff --git a/lib/dml/tests/dml_test.php b/lib/dml/tests/dml_test.php
index a5ad8c25010..2bb7270430c 100644
--- a/lib/dml/tests/dml_test.php
+++ b/lib/dml/tests/dml_test.php
@@ -1137,7 +1137,7 @@ class dml_testcase extends database_driver_testcase {
         $tablename = $table->getName();
 
         $table->add_field('id', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
-        $table->add_field('course', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, null, '0');
+        $table->add_field('course', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, null, null, '0');
         $table->add_index('course', XMLDB_INDEX_NOTUNIQUE, array('course'));
         $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
         $dbman->create_table($table);
@@ -1146,9 +1146,11 @@ class dml_testcase extends database_driver_testcase {
         $DB->insert_record($tablename, array('course' => 3));
         $DB->insert_record($tablename, array('course' => 5));
         $DB->insert_record($tablename, array('course' => 2));
+        $DB->insert_record($tablename, array('course' => null));
+        $DB->insert_record($tablename, array('course' => 1));
+        $DB->insert_record($tablename, array('course' => 0));
 
         $rs = $DB->get_recordset_list($tablename, 'course', array(3, 2));
-
         $counter = 0;
         foreach ($rs as $record) {
             $counter++;
@@ -1156,6 +1158,54 @@ class dml_testcase extends database_driver_testcase {
         $this->assertEquals(3, $counter);
         $rs->close();
 
+        $rs = $DB->get_recordset_list($tablename, 'course', array(3));
+        $counter = 0;
+        foreach ($rs as $record) {
+            $counter++;
+        }
+        $this->assertEquals(2, $counter);
+        $rs->close();
+
+        $rs = $DB->get_recordset_list($tablename, 'course', array(null));
+        $counter = 0;
+        foreach ($rs as $record) {
+            $counter++;
+        }
+        $this->assertEquals(1, $counter);
+        $rs->close();
+
+        $rs = $DB->get_recordset_list($tablename, 'course', array(6, null));
+        $counter = 0;
+        foreach ($rs as $record) {
+            $counter++;
+        }
+        $this->assertEquals(1, $counter);
+        $rs->close();
+
+        $rs = $DB->get_recordset_list($tablename, 'course', array(null, 5, 5, 5));
+        $counter = 0;
+        foreach ($rs as $record) {
+            $counter++;
+        }
+        $this->assertEquals(2, $counter);
+        $rs->close();
+
+        $rs = $DB->get_recordset_list($tablename, 'course', array(true));
+        $counter = 0;
+        foreach ($rs as $record) {
+            $counter++;
+        }
+        $this->assertEquals(1, $counter);
+        $rs->close();
+
+        $rs = $DB->get_recordset_list($tablename, 'course', array(false));
+        $counter = 0;
+        foreach ($rs as $record) {
+            $counter++;
+        }
+        $this->assertEquals(1, $counter);
+        $rs->close();
+
         $rs = $DB->get_recordset_list($tablename, 'course',array()); // Must return 0 rows without conditions. MDL-17645
 
         $counter = 0;