From edb5cd0de4d61e14e58aa4a3d2f9785d8c2be156 Mon Sep 17 00:00:00 2001
From: sam marshall <s.marshall@open.ac.uk>
Date: Mon, 14 Sep 2020 11:23:56 +0100
Subject: [PATCH] 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                           |  2 ++
 4 files changed, 65 insertions(+)

diff --git a/lib/dml/moodle_database.php b/lib/dml/moodle_database.php
index c46ed82d1386c..40a154d9cb49e 100644
--- a/lib/dml/moodle_database.php
+++ b/lib/dml/moodle_database.php
@@ -2038,6 +2038,29 @@ public function delete_records_list($table, $field, array $values) {
         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 4bfa04cb263bf..599913a432609 100644
--- a/lib/dml/mysqli_native_moodle_database.php
+++ b/lib/dml/mysqli_native_moodle_database.php
@@ -1688,6 +1688,23 @@ public function delete_records_select($table, $select, array $params=null) {
         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 fd6c81c3665e0..e23a456d57308 100644
--- a/lib/dml/tests/dml_test.php
+++ b/lib/dml/tests/dml_test.php
@@ -3491,6 +3491,29 @@ public function test_delete_records_select() {
         $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 2d2536ecad794..40aa910902dc9 100644
--- a/lib/upgrade.txt
+++ b/lib/upgrade.txt
@@ -56,6 +56,8 @@ information provided here is intended especially for developers.
   a callback to be provided to determine whether page can be accessed.
 * New setting $CFG->localtempdir overrides which defaults to sys_get_temp_dir()
 * Function redirect() now emits a line of backtrace into the X-Redirect-By header when debugging is on
+* New DML function $DB->delete_records_subquery() to delete records based on a subquery in a way
+  that will work across databases.
 
 === 3.9 ===
 * Following function has been deprecated, please use \core\task\manager::run_from_cli().