From 2cd98f15b3d652b0fb8e2109e02338c421827391 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Mon, 14 Sep 2020 11:23:56 +0100 Subject: [PATCH 1/2] MDL-69687 DB: Add API for deleting data based on subquery The new API works on normal databases (by deleting data based on the subquery) and also on MySQL (by deleting the data using a weird join on the subquery). --- lib/dml/moodle_database.php | 23 +++++++++++++++++++++++ lib/dml/mysqli_native_moodle_database.php | 17 +++++++++++++++++ lib/dml/tests/dml_test.php | 23 +++++++++++++++++++++++ lib/upgrade.txt | 4 ++++ 4 files changed, 67 insertions(+) diff --git a/lib/dml/moodle_database.php b/lib/dml/moodle_database.php index 021706be01a..c0b574ddf56 100644 --- a/lib/dml/moodle_database.php +++ b/lib/dml/moodle_database.php @@ -1954,6 +1954,29 @@ abstract class moodle_database { return $this->delete_records_select($table, $select, $params); } + /** + * Deletes records from a table using a subquery. The subquery should return a list of values + * in a single column, which match one field from the table being deleted. + * + * The $alias parameter must be set to the name of the single column in your subquery result + * (e.g. if the subquery is 'SELECT id FROM whatever', then it should be 'id'). This is not + * needed on most databases, but MySQL requires it. + * + * (On database where the subquery is inefficient, it is implemented differently.) + * + * @param string $table Table to delete from + * @param string $field Field in table to match + * @param string $alias Name of single column in subquery e.g. 'id' + * @param string $subquery Subquery that will return values of the field to delete + * @param array $params Parameters for subquery + * @throws dml_exception If there is any error + * @since Moodle 3.10 + */ + public function delete_records_subquery(string $table, string $field, string $alias, + string $subquery, array $params = []): void { + $this->delete_records_select($table, $field . ' IN (' . $subquery . ')', $params); + } + /** * Delete one or more records from a table which match a particular WHERE clause. * diff --git a/lib/dml/mysqli_native_moodle_database.php b/lib/dml/mysqli_native_moodle_database.php index 8469c0d5003..69143936174 100644 --- a/lib/dml/mysqli_native_moodle_database.php +++ b/lib/dml/mysqli_native_moodle_database.php @@ -1662,6 +1662,23 @@ class mysqli_native_moodle_database extends moodle_database { return true; } + /** + * Deletes records using a subquery, which is done with a strange DELETE...JOIN syntax in MySQL + * because it performs very badly with normal subqueries. + * + * @param string $table Table to delete from + * @param string $field Field in table to match + * @param string $alias Name of single column in subquery e.g. 'id' + * @param string $subquery Query that will return values of the field to delete + * @param array $params Parameters for query + * @throws dml_exception If there is any error + */ + public function delete_records_subquery(string $table, string $field, string $alias, string $subquery, array $params = []): void { + // Aliases mysql_deltable and mysql_subquery are chosen to be unlikely to conflict. + $this->execute("DELETE mysql_deltable FROM {" . $table . "} mysql_deltable JOIN " . + "($subquery) mysql_subquery ON mysql_subquery.$alias = mysql_deltable.$field", $params); + } + public function sql_cast_char2int($fieldname, $text=false) { return ' CAST(' . $fieldname . ' AS SIGNED) '; } diff --git a/lib/dml/tests/dml_test.php b/lib/dml/tests/dml_test.php index a87c36239af..6e472b59bad 100644 --- a/lib/dml/tests/dml_test.php +++ b/lib/dml/tests/dml_test.php @@ -3429,6 +3429,29 @@ class core_dml_testcase extends database_driver_testcase { $this->assertEquals(1, $DB->count_records($tablename)); } + public function test_delete_records_subquery() { + $DB = $this->tdb; + $dbman = $DB->get_manager(); + + $table = $this->get_test_table(); + $tablename = $table->getName(); + + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('course', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0'); + $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); + $dbman->create_table($table); + + $DB->insert_record($tablename, array('course' => 3)); + $DB->insert_record($tablename, array('course' => 2)); + $DB->insert_record($tablename, array('course' => 2)); + + // This is not a useful scenario for using a subquery, but it will be sufficient for testing. + // Use the 'frog' alias just to make it clearer when we are testing the alias parameter. + $DB->delete_records_subquery($tablename, 'id', 'frog', + 'SELECT id AS frog FROM {' . $tablename . '} WHERE course = ?', [2]); + $this->assertEquals(1, $DB->count_records($tablename)); + } + public function test_delete_records_list() { $DB = $this->tdb; $dbman = $DB->get_manager(); diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 709dee47def..0a363187c4b 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -1,6 +1,10 @@ This files describes API changes in core libraries and APIs, information provided here is intended especially for developers. +=== 3.8.6 === +* New DML function $DB->delete_records_subquery() to delete records based on a subquery in a way + that will work across databases. + === 3.8.5 === * The `$CFG->behat_retart_browser_after` configuration setting has been removed. The browser session is now restarted between all tests. From 66de9e27becab1bc031ecacab9eec2b95ecd6a39 Mon Sep 17 00:00:00 2001 From: sam marshall Date: Mon, 5 Oct 2020 10:27:38 +0100 Subject: [PATCH 2/2] MDL-69687 Course: Improve removal of completion data on MySQL --- lib/moodlelib.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/moodlelib.php b/lib/moodlelib.php index ee4c4b0fc87..d4c5e583373 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -5277,9 +5277,8 @@ function remove_course_contents($courseid, $showfeedback = true, array $options // Remove all data from availability and completion tables that is associated // with course-modules belonging to this course. Note this is done even if the // features are not enabled now, in case they were enabled previously. - $DB->delete_records_select('course_modules_completion', - 'coursemoduleid IN (SELECT id from {course_modules} WHERE course=?)', - array($courseid)); + $DB->delete_records_subquery('course_modules_completion', 'coursemoduleid', 'id', + 'SELECT id from {course_modules} WHERE course = ?', [$courseid]); // Remove course-module data that has not been removed in modules' _delete_instance callbacks. $cms = $DB->get_records('course_modules', array('course' => $course->id));