MDL-68509 core_ddl: Keep databasemeta cache after dropping temptables

Backport of MDL-58584 (934c2ee + 94e2509)
moodle · Apr 24, 2020 · 4281fe1 · 4281fe1
1 parent a6f315c
commit 4281fe1
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 33 deletions.
diff --git a/lib/ddl/database_manager.php b/lib/ddl/database_manager.php
@@ -330,6 +330,8 @@ public function drop_table(xmldb_table $xmldb_table) {
             throw new ddl_exception('ddlunknownerror', null, 'table drop sql not generated');
         }
         $this->execute_sql_arr($sqlarr, array($xmldb_table->getName()));
+
+        $this->generator->cleanup_after_drop($xmldb_table);
     }
 
     /**

diff --git a/lib/ddl/mssql_sql_generator.php b/lib/ddl/mssql_sql_generator.php
@@ -183,21 +183,6 @@ public function getCreateTempTableSQL($xmldb_table) {
         return $sqlarr;
     }
 
-    /**
-     * Given one correct xmldb_table, returns the SQL statements
-     * to drop it (inside one array).
-     *
-     * @param xmldb_table $xmldb_table The table to drop.
-     * @return array SQL statement(s) for dropping the specified table.
-     */
-    public function getDropTableSQL($xmldb_table) {
-        $sqlarr = parent::getDropTableSQL($xmldb_table);
-        if ($this->temptables->is_temptable($xmldb_table->getName())) {
-            $this->temptables->delete_temptable($xmldb_table->getName());
-        }
-        return $sqlarr;
-    }
-
     /**
      * Given one XMLDB Type, length and decimals, returns the DB proper SQL type.
      *

diff --git a/lib/ddl/mysql_sql_generator.php b/lib/ddl/mysql_sql_generator.php
@@ -384,7 +384,6 @@ public function getDropTableSQL($xmldb_table) {
         $sqlarr = parent::getDropTableSQL($xmldb_table);
         if ($this->temptables->is_temptable($xmldb_table->getName())) {
             $sqlarr = preg_replace('/^DROP TABLE/', "DROP TEMPORARY TABLE", $sqlarr);
-            $this->temptables->delete_temptable($xmldb_table->getName());
         }
         return $sqlarr;
     }

diff --git a/lib/ddl/oracle_sql_generator.php b/lib/ddl/oracle_sql_generator.php
@@ -211,7 +211,6 @@ public function getDropTableSQL($xmldb_table) {
         $sqlarr = parent::getDropTableSQL($xmldb_table);
         if ($this->temptables->is_temptable($xmldb_table->getName())) {
             array_unshift($sqlarr, "TRUNCATE TABLE ". $this->getTableName($xmldb_table)); // oracle requires truncate before being able to drop a temp table
-            $this->temptables->delete_temptable($xmldb_table->getName());
         }
         return $sqlarr;
     }

diff --git a/lib/ddl/postgres_sql_generator.php b/lib/ddl/postgres_sql_generator.php
@@ -103,21 +103,6 @@ public function getCreateTempTableSQL($xmldb_table) {
         return $sqlarr;
     }
 
-    /**
-     * Given one correct xmldb_table, returns the SQL statements
-     * to drop it (inside one array).
-     *
-     * @param xmldb_table $xmldb_table The table to drop.
-     * @return array SQL statement(s) for dropping the specified table.
-     */
-    public function getDropTableSQL($xmldb_table) {
-        $sqlarr = parent::getDropTableSQL($xmldb_table);
-        if ($this->temptables->is_temptable($xmldb_table->getName())) {
-            $this->temptables->delete_temptable($xmldb_table->getName());
-        }
-        return $sqlarr;
-    }
-
     /**
      * Given one correct xmldb_index, returns the SQL statements
      * needed to create it (in array).

diff --git a/lib/ddl/sql_generator.php b/lib/ddl/sql_generator.php
@@ -653,7 +653,7 @@ public function getRenameTableSQL($xmldb_table, $newname) {
     }
 
     /**
-     * Given one correct xmldb_table and the new name, returns the SQL statements
+     * Given one correct xmldb_table, returns the SQL statements
      * to drop it (inside one array). Works also for temporary tables.
      *
      * @param xmldb_table $xmldb_table The table to drop.
@@ -674,6 +674,17 @@ public function getDropTableSQL($xmldb_table) {
         return $results;
     }
 
+    /**
+     * Performs any clean up that needs to be done after a table is dropped.
+     *
+     * @param xmldb_table $table
+     */
+    public function cleanup_after_drop(xmldb_table $table): void {
+        if ($this->temptables->is_temptable($table->getName())) {
+            $this->temptables->delete_temptable($table->getName());
+        }
+    }
+
     /**
      * Given one xmldb_table and one xmldb_field, return the SQL statements needed to add the field to the table.
      *

diff --git a/lib/ddl/tests/ddl_test.php b/lib/ddl/tests/ddl_test.php
@@ -1828,6 +1828,76 @@ public function test_concurrent_temp_tables() {
         $this->assertFalse($dbman->table_exists('test_table1'));
     }
 
+    /**
+     * get_columns should return an empty array for ex-temptables.
+     */
+    public function test_leftover_temp_tables_columns() {
+        $DB = $this->tdb; // Do not use global $DB!
+        $dbman = $this->tdb->get_manager();
+
+        // Create temp table0.
+        $table0 = $this->tables['test_table0'];
+        $dbman->create_temp_table($table0);
+
+        $dbman->drop_table($table0);
+
+        // Get columns and perform some basic tests.
+        $columns = $DB->get_columns('test_table0');
+        $this->assertEquals([], $columns);
+    }
+
+    /**
+     * Deleting a temp table should not purge the whole cache
+     */
+    public function test_leftover_temp_tables_cache() {
+        $DB = $this->tdb; // Do not use global $DB!
+        $dbman = $this->tdb->get_manager();
+
+        // Create 2 temp tables.
+        $table0 = $this->tables['test_table0'];
+        $dbman->create_temp_table($table0);
+        $table1 = $this->tables['test_table1'];
+        $dbman->create_temp_table($table1);
+
+        // Create a normal table.
+        $table2 = new xmldb_table ('test_table2');
+        $table2->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
+        $table2->add_field('course', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0');
+        $table2->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+        $table2->setComment("This is a test'n drop table. You can drop it safely");
+        $this->tables[$table2->getName()] = $table2;
+        $dbman->create_table($table2);
+
+        // Get columns for the tables, so that relevant caches are populated with their data.
+        $DB->get_columns('test_table0');
+        $DB->get_columns('test_table1');
+        $DB->get_columns('test_table2');
+
+        $dbman->drop_table($table0);
+
+        $rc = new ReflectionClass('moodle_database');
+        $rcm = $rc->getMethod('get_temp_tables_cache');
+        $rcm->setAccessible(true);
+        $metacachetemp = $rcm->invokeArgs($DB, []);
+
+        // Data of test_table0 should be removed from the cache.
+        $this->assertEquals(false, $metacachetemp->has('test_table0'));
+
+        // Data of test_table1 should be intact.
+        $this->assertEquals(true, $metacachetemp->has('test_table1'));
+
+        $rc = new ReflectionClass('moodle_database');
+        $rcm = $rc->getMethod('get_metacache');
+        $rcm->setAccessible(true);
+        $metacache = $rcm->invokeArgs($DB, []);
+
+        // Data of test_table2 should be intact.
+        $this->assertEquals(true, $metacache->has('test_table2'));
+
+        // Delete the leftover temp table.
+        $dbman->drop_table($table1);
+    }
+
     public function test_reset_sequence() {
         $DB = $this->tdb;
         $dbman = $DB->get_manager();

diff --git a/lib/upgrade.txt b/lib/upgrade.txt
@@ -3,6 +3,7 @@ information provided here is intended especially for developers.
 
 === 3.8.3 ===
 * database_manager::check_database_schema() now checks for missing indexes.
+* Added function cleanup_after_drop to the database_manager class to take care of all the cleanups that need to be done after a table is dropped.
 
 === 3.8.2 ===
 * grade_item::update_final_grade() can now take an optional parameter to set the grade->timemodified. If not present the current time will carry on being used.