Skip to content

Commit

Permalink
MDL-34159 improve where_clause_list performance
Browse files Browse the repository at this point in the history
  • Loading branch information
@skodak
skodak committed Jul 6, 2012
1 parent f2867a8 commit 6128bf9
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 18 deletions.
37 changes: 21 additions & 16 deletions lib/dml/moodle_database.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,21 +574,38 @@ protected function where_clause($table, array $conditions=null) {
* @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);
}

Expand Down Expand Up @@ -1034,10 +1051,6 @@ public function get_recordset($table, array $conditions=null, $sort='', $fields=
*/
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);
}

Expand Down Expand Up @@ -1132,10 +1145,6 @@ public function get_records($table, array $conditions=null, $sort='', $fields='*
*/
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);
}

Expand Down Expand Up @@ -1662,10 +1671,6 @@ public function delete_records($table, array $conditions=null) {
*/
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);
}

Expand Down
54 changes: 52 additions & 2 deletions lib/dml/tests/dml_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,7 @@ public function test_get_recordset_list() {
$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);
Expand All @@ -1146,16 +1146,66 @@ public function test_get_recordset_list() {
$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++;
}
$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;
Expand Down

0 comments on commit 6128bf9

Please sign in to comment.