MDL-73727 core_files: Add subquery to avoid a seq scan on larger sites

- Formatted the query to meet SQL coding styles - Add index on {files} to prevent performance regression - Fork the SQL query based on DB family based on support
agwells · May 30, 2022 · f984566 · f984566
1 parent 117b240
commit f984566
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 12 deletions.
diff --git a/files/classes/conversion.php b/files/classes/conversion.php
@@ -128,16 +128,36 @@ public static function get_conversions_for_file(stored_file $file, $format) {
 
         // Fetch actual conversions which relate to the specified source file, and have a matching conversion record,
         // and either have a valid destination file which still exists, or do not have a destination file at all.
-        $sql = "SELECT {$sqlfields}
-                FROM {" . self::TABLE . "} c
-                INNER JOIN {files} conversionsourcefile ON conversionsourcefile.id = c.sourcefileid
-                LEFT JOIN {files} conversiondestfile ON conversiondestfile.id = c.destfileid
-                WHERE
-                    conversionsourcefile.contenthash = :ccontenthash
-                AND c.targetformat = :cformat
-                AND (
-                    c.destfileid IS NULL OR conversiondestfile.id IS NOT NULL
-                )";
+        $dbfamily = $DB->get_dbfamily();
+        switch ($dbfamily) {
+            // For certain DB engines, use a more optimised query.
+            case 'mysql':
+            case 'postgres':
+                $sql = "SELECT {$sqlfields}
+                          FROM {" . self::TABLE . "} c
+                          JOIN (SELECT id
+                                  FROM {files}
+                                 WHERE contenthash = :ccontenthash
+                                 LIMIT 1
+                               ) conversionsourcefile ON conversionsourcefile.id = c.sourcefileid
+                     LEFT JOIN {files} conversiondestfile ON conversiondestfile.id = c.destfileid
+                         WHERE c.targetformat = :cformat
+                           AND (c.destfileid IS NULL
+                                OR conversiondestfile.id IS NOT NULL)";
+                break;
+
+            // For everything else, use the standard cross-db compatible query.
+            default:
+                $sql = "SELECT {$sqlfields}
+                          FROM {" . self::TABLE . "} c
+                    INNER JOIN {files} conversionsourcefile ON conversionsourcefile.id = c.sourcefileid
+                     LEFT JOIN {files} conversiondestfile ON conversiondestfile.id = c.destfileid
+                         WHERE conversionsourcefile.contenthash = :ccontenthash
+                           AND c.targetformat = :cformat
+                           AND (c.destfileid IS NULL
+                                OR conversiondestfile.id IS NOT NULL)";
+                break;
+        }
 
         // Fetch a empty conversion record for each source/destination combination that we find to match where the
         // destination file is in the correct filearea/filepath/filename combination to meet the requirements.

diff --git a/lib/db/install.xml b/lib/db/install.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="lib/db" VERSION="20220510" COMMENT="XMLDB file for core Moodle tables"
+<XMLDB PATH="lib/db" VERSION="20220530" COMMENT="XMLDB file for core Moodle tables"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../lib/xmldb/xmldb.xsd"
 >
@@ -2600,6 +2600,7 @@
         <INDEX NAME="contenthash" UNIQUE="false" FIELDS="contenthash"/>
         <INDEX NAME="pathnamehash" UNIQUE="true" FIELDS="pathnamehash"/>
         <INDEX NAME="license" UNIQUE="false" FIELDS="license"/>
+        <INDEX NAME="filename" UNIQUE="false" FIELDS="filename"/>
       </INDEXES>
     </TABLE>
     <TABLE NAME="files_reference" COMMENT="Store files references">

diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php
@@ -4515,5 +4515,20 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2022052500.00);
     }
 
+    if ($oldversion < 2022053000.00) {
+
+        // Define index filename (not unique) to be added to files.
+        $table = new xmldb_table('files');
+        $index = new xmldb_index('filename', XMLDB_INDEX_NOTUNIQUE, ['filename']);
+
+        // Conditionally launch add index filename.
+        if (!$dbman->index_exists($table, $index)) {
+            $dbman->add_index($table, $index);
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2022053000.00);
+    }
+
     return true;
 }
diff --git a/version.php b/version.php
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2022052700.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2022053000.00;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.
 $release  = '4.1dev (Build: 20220527)'; // Human-friendly version name