MDL-58584 core_ddl: getDropTableSQL should not do more than what it says

This commit is contained in:
Shamim Rezaie 2020-01-22 12:21:30 +11:00
parent b4d062c6be
commit 934c2ee1b6
8 changed files with 83 additions and 33 deletions

View File

@ -330,6 +330,8 @@ class database_manager {
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);
}
/**

View File

@ -183,21 +183,6 @@ class mssql_sql_generator extends sql_generator {
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.
*

View File

@ -384,7 +384,6 @@ class mysql_sql_generator extends sql_generator {
$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;
}

View File

@ -211,7 +211,6 @@ class oracle_sql_generator extends sql_generator {
$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;
}

View File

@ -103,21 +103,6 @@ class postgres_sql_generator extends sql_generator {
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).

View File

@ -653,7 +653,7 @@ abstract class sql_generator {
}
/**
* 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 @@ abstract class sql_generator {
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.
*

View File

@ -1828,6 +1828,74 @@ class core_ddl_testcase extends database_driver_testcase {
$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'));
}
public function test_reset_sequence() {
$DB = $this->tdb;
$dbman = $DB->get_manager();

View File

@ -30,6 +30,7 @@ information provided here is intended especially for developers.
- mod_quiz
* The database drivers (moodle_database and subclasses) don't need to implement get_columns() anymore.
They have to implement fetch_columns instead.
* 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 ===
* Add CLI option to notify all cron tasks to stop: admin/cli/cron.php --stop