MDL-67886 core: check_database_schema() checks for missing indexes

lib/ddl/database_manager.php

@@ -962,6 +962,7 @@ class database_manager {
             'extracolumns' => true,
             'missingcolumns' => true,
             'changedcolumns' => true,
+            'missingindexes' => true
         );
 
         $typesmap = array(
@@ -1096,6 +1097,46 @@ class database_manager {
                 unset($dbfields[$fieldname]);
             }
 
+            // Check for missing indexes/keys.
+            if ($options['missingindexes']) {
+                // Check the foreign keys.
+                if ($keys = $table->getKeys()) {
+                    foreach ($keys as $key) {
+                        // Primary keys are skipped.
+                        if ($key->getType() == XMLDB_KEY_PRIMARY) {
+                            continue;
+                        }
+
+                        $keyname = $key->getName();
+
+                        // Create the interim index.
+                        $index = new xmldb_index('anyname');
+                        $index->setFields($key->getFields());
+                        switch ($key->getType()) {
+                            case XMLDB_KEY_UNIQUE:
+                            case XMLDB_KEY_FOREIGN_UNIQUE:
+                                $index->setUnique(true);
+                                break;
+                            case XMLDB_KEY_FOREIGN:
+                                $index->setUnique(false);
+                                break;
+                        }
+                        if (!$this->index_exists($table, $index)) {
+                            $errors[$tablename][] = $this->get_missing_index_error($table, $index, $keyname);
+                        }
+                    }
+                }
+
+                // Check the indexes.
+                if ($indexes = $table->getIndexes()) {
+                    foreach ($indexes as $index) {
+                        if (!$this->index_exists($table, $index)) {
+                            $errors[$tablename][] = $this->get_missing_index_error($table, $index, $index->getName());
+                        }
+                    }
+                }
+            }
+
             // Check for extra columns (indicates unsupported hacks) - modify install.xml if you want to pass validation.
             foreach ($dbfields as $fieldname => $dbfield) {
                 if ($options['extracolumns']) {
@@ -1127,4 +1168,20 @@ class database_manager {
 
         return $errors;
     }
+
+    /**
+     * Returns a string describing the missing index error.
+     *
+     * @param xmldb_table $table
+     * @param xmldb_index $index
+     * @param string $indexname
+     * @return string
+     */
+    private function get_missing_index_error(xmldb_table $table, xmldb_index $index, string $indexname): string {
+        $sqlarr = $this->generator->getAddIndexSQL($table, $index);
+        $sqlarr = $this->generator->getEndedStatements($sqlarr);
+        $sqltoadd = reset($sqlarr);
+
+        return "Missing index '" . $indexname . "' " . "(" . $index->readableInfo() . "). \n" . $sqltoadd;
+    }
 }

lib/ddl/tests/ddl_test.php

@@ -2441,4 +2441,52 @@ class core_ddl_testcase extends database_driver_testcase {
         }
     */
 
+    /**
+     * Tests check_database_schema().
+     */
+    public function test_check_database_schema() {
+        global $CFG, $DB;
+
+        $dbmanager = $DB->get_manager();
+
+        // Create a table in the database we will be using to compare with a schema.
+        $table = new xmldb_table('test_check_db_schema');
+        $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
+        $table->add_field('extracolumn', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
+        $table->add_field('courseid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
+        $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+        $table->setComment("This is a test table, you can drop it safely.");
+        $dbmanager->create_table($table);
+
+        // Remove the column so it is not added to the schema and gets reported as an extra column.
+        $table->deleteField('extracolumn');
+
+        // Change the 'courseid' field to a float in the schema so it gets reported as different.
+        $table->deleteField('courseid');
+        $table->add_field('courseid', XMLDB_TYPE_NUMBER, '10, 2', null, XMLDB_NOTNULL, null, null);
+
+        // Add another column to the schema that won't be present in the database and gets reported as missing.
+        $table->add_field('missingcolumn', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
+
+        // Add another key to the schema that won't be present in the database and gets reported as missing.
+        $table->add_key('missingkey', XMLDB_KEY_FOREIGN, array('courseid'), 'course', array('id'));
+
+        $schema = new xmldb_structure('testschema');
+        $schema->addTable($table);
+
+        // Things we want to check for -
+        // 1. Changed columns.
+        // 2. Missing columns.
+        // 3. Missing indexes.
+        // 4. Extra columns.
+        $errors = $dbmanager->check_database_schema($schema)['test_check_db_schema'];
+        $strmissing = "Missing index 'missingkey' (not unique (courseid)). " . PHP_EOL .
+            "CREATE INDEX {$CFG->prefix}testchecdbsche_cou_ix ON {$CFG->prefix}test_check_db_schema (courseid);";
+        $this->assertCount(4, $errors);
+
+        $this->assertContains("column 'courseid' has incorrect type 'I', expected 'N'", $errors);
+        $this->assertContains("column 'missingcolumn' is missing", $errors);
+        $this->assertContains($strmissing, $errors);
+        $this->assertContains("column 'extracolumn' is not expected (I)", $errors);
+    }
 }

lib/dtl/database_exporter.php

@@ -129,7 +129,10 @@ abstract class database_exporter {
     public function export_database($description=null) {
         global $CFG;
 
-        $options = array('changedcolumns' => false); // Column types may be fixed by transfer.
+        $options = [
+            'changedcolumns' => false, // Column types may be fixed by transfer.
+            'missingindexes' => false // No need to worry about indexes for transfering data.
+        ];
         if ($this->check_schema and $errors = $this->manager->check_database_schema($this->schema, $options)) {
             $details = '';
             foreach ($errors as $table=>$items) {

lib/dtl/database_importer.php

@@ -110,7 +110,10 @@ class database_importer {
             throw new dbtransfer_exception('importversionmismatchexception', $a);
         }
 
-        $options = array('changedcolumns' => false); // Column types may be fixed by transfer.
+        $options = [
+            'changedcolumns' => false, // Column types may be fixed by transfer.
+            'missingindexes' => false // No need to worry about indexes for transfering data.
+        ];
         if ($this->check_schema and $errors = $this->manager->check_database_schema($this->schema, $options)) {
             $details = '';
             foreach ($errors as $table=>$items) {

lib/upgrade.txt

@@ -38,6 +38,7 @@ information provided here is intended especially for developers.
 * H5P libraries have been moved from /lib/h5p to h5p/h5plib as an h5plib plugintype.
 * mdn-polyfills has been renamed to polyfills. The reason there is no polyfill from the MDN is
   because there is no example polyfills on the MDN for this functionality.
+* database_manager::check_database_schema() now checks for missing indexes.
 
 === 3.8 ===
 * Add CLI option to notify all cron tasks to stop: admin/cli/cron.php --stop