Replace flag test_name with foreign key to test_names (aces#9338)

There is a varchar for every row in the flag table, this normalizes it by replacing it with a foreign key to the test_name tables that is already present and populated. This both makes table scans of the flag table faster and is just better schema design.
regisoc · Sep 20, 2024 · 0413aaf · 0413aaf
1 parent 4c178f9
commit 0413aaf
Show file tree

Hide file tree

Showing 18 changed files with 5,699 additions and 5,653 deletions.
diff --git a/SQL/0000-00-00-schema.sql b/SQL/0000-00-00-schema.sql
@@ -292,7 +292,7 @@ CREATE TABLE `instrument_data` (
 CREATE TABLE `flag` (
   `ID` int(10) unsigned NOT NULL auto_increment,
   `SessionID` int(10) unsigned NOT NULL,
-  `Test_name` varchar(255) NOT NULL default '',
+  `TestID` int(10) unsigned NOT NULL,
   `CommentID` varchar(255) NOT NULL default '',
   `Data_entry` enum('In Progress','Complete') default NULL,
   `Required_elements_completed` enum('Y','N') NOT NULL default 'N',
@@ -305,15 +305,14 @@ CREATE TABLE `flag` (
   PRIMARY KEY  (`CommentID`),
   KEY `flag_ID` (`ID`),
   KEY `flag_SessionID` (`SessionID`),
-  KEY `flag_Test_name` (`Test_name`),
   KEY `flag_Exclusion` (`Exclusion`),
   KEY `flag_Data_entry` (`Data_entry`),
   KEY `flag_Validity` (`Validity`),
   KEY `flag_Administration` (`Administration`),
   KEY `flag_UserID` (`UserID`),
   CONSTRAINT `FK_flag_1` FOREIGN KEY (`SessionID`) REFERENCES `session` (`ID`) ON DELETE CASCADE ON UPDATE CASCADE,
-  CONSTRAINT `FK_flag_2` FOREIGN KEY (`Test_name`) REFERENCES `test_names` (`Test_name`),
-  CONSTRAINT `FK_flag_3` FOREIGN KEY (`DataID`) REFERENCES `instrument_data` (`ID`)
+  CONSTRAINT `FK_flag_3` FOREIGN KEY (`DataID`) REFERENCES `instrument_data` (`ID`),
+  CONSTRAINT `FK_ibfk_1` FOREIGN KEY (`TestID`) REFERENCES `test_names` (`ID`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
 CREATE TABLE `history` (

diff --git a/SQL/Cleanup_patches/2024-09-13-NoTestName.sql b/SQL/Cleanup_patches/2024-09-13-NoTestName.sql
@@ -0,0 +1,2 @@
+ALTER TABLE flag DROP CONSTRAINT FK_flag_2;
+ALTER TABLE flag DROP COLUMN test_name;
diff --git a/SQL/New_patches/2024-09-13-TestID.sql b/SQL/New_patches/2024-09-13-TestID.sql
@@ -0,0 +1,6 @@
+ALTER TABLE flag ADD COLUMN TestID int(10) unsigned AFTER test_name;
+ALTER TABLE flag ADD CONSTRAINT FOREIGN KEY (TestID) REFERENCES test_names(ID);
+
+UPDATE flag f SET TestID=(SELECT ID FROM test_names tn WHERE f.test_name=tn.test_name);
+
+ALTER TABLE flag MODIFY COLUMN TestID int(10) unsigned NOT NULL;
diff --git a/modules/bvl_feedback/php/module.class.inc b/modules/bvl_feedback/php/module.class.inc
@@ -113,7 +113,9 @@ class Module extends \Module
 
                 case 'instrument':
                     $instrument = $DB->pselectOne(
-                        "SELECT Test_name from flag WHERE CommentID=:cid",
+                        "SELECT Test_name from flag f
+			    	JOIN test_names tn ON (f.TestID=tn.ID)
+			    	WHERE CommentID=:cid",
                         ['cid' => $row['CommentID']]
                     );
                     if (!empty($instrument)) {

diff --git a/modules/conflict_resolver/php/module.class.inc b/modules/conflict_resolver/php/module.class.inc
@@ -139,15 +139,16 @@ class Module extends \Module
 
             $candID  = $candidate->getCandID();
             $results = $DB->pselect(
-                "SELECT f1.Test_name, s1.Visit_label, COUNT(*) as Conflicts
+                "SELECT tn.Test_name, s1.Visit_label, COUNT(*) as Conflicts
                  FROM conflicts_unresolved cu
                      LEFT JOIN flag f1 ON (f1.CommentID=cu.CommentId1)
+                     JOIN test_names tn ON (f1.TestID=tn.ID)
                      LEFT JOIN flag f2 ON (f2.CommentID=cu.CommentID2)
                      LEFT JOIN session s1 ON (f1.SessionID=s1.ID)
                      LEFT JOIN session s2 ON (f2.SessionID=s2.ID)
                  WHERE (s1.CandID=:cand1 OR s2.CandID=:cand2)
-                 GROUP BY f1.Test_name, s1.Visit_label
-                 ORDER BY s1.Visit_label, f1.Test_name",
+                 GROUP BY tn.Test_name, s1.Visit_label
+                 ORDER BY s1.Visit_label, tn.Test_name",
                 ['cand1' => $candID, 'cand2' => $candID],
             );
             // Do not show the conflicts card if there are no conflicts

diff --git a/modules/dashboard/test/DashboardTest.php b/modules/dashboard/test/DashboardTest.php
@@ -136,7 +136,7 @@ function setUp(): void
             [
                 'ID'         => '111111',
                 'SessionID'  => '222222',
-                'Test_name'  => 'TestName11111111111',
+                'TestID'     => '111',
                 'CommentID'  => 'commentID111',
                 'Data_entry' => 'In Progress',
             ]

diff --git a/modules/imaging_browser/php/imaging_session_controlpanel.class.inc b/modules/imaging_browser/php/imaging_session_controlpanel.class.inc
@@ -85,8 +85,8 @@ class Imaging_Session_ControlPanel
         foreach ($linkedInstruments as $v) {
             $qresult = $DB->pselectRow(
                 "SELECT f.CommentID, tn.Full_name FROM flag f
-            JOIN test_names tn ON f.Test_name = tn.Test_name
-            WHERE f.Test_name = :tname AND f.SessionID = :v_sessions_id
+            JOIN test_names tn ON f.TestID = tn.ID
+            WHERE tn.Test_name = :tname AND f.SessionID = :v_sessions_id
             AND f.CommentID NOT LIKE 'DDE_%'",
                 [
                     'tname'         => $v,
@@ -100,7 +100,7 @@ class Imaging_Session_ControlPanel
                     "CommentID" => $qresult['CommentID'],
                 ];
             }
-            // else 'Could not find linked instruments';
+                   // else 'Could not find linked instruments';
         }
         $subjectData['links'] = $links;
 

diff --git a/modules/instruments/php/instrumentqueryengine.class.inc b/modules/instruments/php/instrumentqueryengine.class.inc
@@ -106,9 +106,10 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine
 
         $query       = "SELECT c.CandID, f.CommentID
             FROM flag f 
+		JOIN test_names tn ON (f.TestID=tn.ID)
             JOIN session s ON (s.ID=f.SessionID AND s.Active='Y')
             JOIN candidate c ON (s.CandID=c.CandID AND c.Active='Y')
-            WHERE Test_name=:tn AND f.CommentID NOT LIKE 'DDE%'";
+            WHERE tn.Test_name=:tn AND f.CommentID NOT LIKE 'DDE%'";
         $queryparams = ['tn' => $testname];
         if ($visitlist !== null) {
             $query .= ' AND s.Visit_label IN (';
@@ -265,9 +266,10 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine
         $visits = $DB->pselectCol(
             "SELECT DISTINCT s.Visit_Label
                 FROM flag f
+                   JOIN test_names tn ON (tn.ID=f.TestID)
                    JOIN session s ON (f.SessionID=s.ID)
                    JOIN candidate c ON (c.CandID=s.CandID)
-                WHERE s.Active='Y' AND c.Active='Y' and f.Test_name=:tn
+                WHERE s.Active='Y' AND c.Active='Y' and tn.Test_name=:tn
             ORDER BY s.Visit_label",
             ['tn' => $inst->getName()]
         );
@@ -336,11 +338,12 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine
         $rows = iterator_to_array(
             $DB->pselect(
                 "SELECT c.CandID, CommentID FROM flag f 
+		JOIN test_names tn ON (f.TestID=tn.ID)
                 JOIN session s ON (f.SessionID=s.ID)
                 JOIN candidate c ON (s.CandID=c.CandID)
              WHERE f.CommentID NOT LIKE 'DDE%'
                 AND c.CandID IN (SELECT CandID FROM querycandidates)
-                AND f.Test_name IN ('" . join("', '", $instruments). "')
+                AND tn.Test_name IN ('" . join("', '", $instruments). "')
                 AND c.Active='Y' AND s.Active='Y'
              ORDER BY c.CandID",
                 [],

diff --git a/modules/instruments/php/visitsummary.class.inc b/modules/instruments/php/visitsummary.class.inc
@@ -77,11 +77,12 @@ class VisitSummary extends \NDB_Page
 
         $bvl_result = $DB->pselect(
             "SELECT
-                f.Test_name,
+                tn.Test_name,
                 f.CommentID,
                 COUNT(cu.ConflictID) AS NumOfConflict
             FROM session s
                 LEFT JOIN flag f ON (s.ID = f.SessionID)
+                	JOIN test_names tn ON (tn.ID = f.TestID)
                 LEFT JOIN conflicts_unresolved cu
                     ON (cu.CommentId1=f.CommentID OR cu.CommentID2=f.CommentID)
                 LEFT JOIN candidate c USING (CandID)
@@ -92,13 +93,13 @@ class VisitSummary extends \NDB_Page
                 s.Visit_label,
                 s.ID,
                 s.Date_visit,
-                f.Test_name,
+                tn.Test_name,
                 f.CommentID,
                 s.Current_stage,
                 s.Screening,
                 s.Visit,
                 s.Approval
-            ORDER BY s.Visit_label, f.Test_name, f.CommentID",
+            ORDER BY s.Visit_label, tn.Test_name, f.CommentID",
             ['candid' => $candid, 'vl' => $visitLabel]
         );
 

diff --git a/php/libraries/NDB_BVL_Battery.class.inc b/php/libraries/NDB_BVL_Battery.class.inc
@@ -229,12 +229,16 @@ class NDB_BVL_Battery
             $DB->insert($obj->table, ['CommentID' => $ddeCommentID]);
         }
 
+        $testId = $DB->pselectOne(
+            "SELECT ID FROM test_names WHERE Test_name=:tn",
+            ['tn' => $testName]
+        );
         // insert into the flag table
         $DB->insert(
             'flag',
             [
                 'SessionID' => $sessionData['SessionID'],
-                'Test_name' => $testName,
+                'TestID'    => $testId,
                 'CommentID' => $commentID,
                 'UserID'    => $sessionData['UserID'],
             ]
@@ -245,7 +249,7 @@ class NDB_BVL_Battery
             'flag',
             [
                 'SessionID' => $sessionData['SessionID'],
-                'Test_name' => $testName,
+                'TestID'    => $testId,
                 'CommentID' => $ddeCommentID,
                 'UserID'    => $sessionData['UserID'],
             ]
@@ -286,7 +290,9 @@ class NDB_BVL_Battery
         $this->_assertBatterySelected();
 
         // craft the select query
-        $query   = "SELECT f.Test_name FROM flag f JOIN session s
+        $query   = "SELECT tn.Test_name FROM flag f
+		JOIN test_names tn ON (f.TestID=tn.ID)
+		JOIN session s
             ON (s.ID=f.SessionID)
             JOIN candidate c ON (c.CandID=s.CandID)
             WHERE f.SessionID=:SID AND s.Active='Y' AND c.Active='Y'
@@ -318,7 +324,7 @@ class NDB_BVL_Battery
         $this->_assertBatterySelected();
 
         // craft the select query
-        $query = "SELECT f.Test_name,
+        $query = "SELECT t.Test_name,
                       t.Full_name,
                       f.CommentID,
                       CONCAT('DDE_', f.CommentID) AS DDECommentID,
@@ -327,7 +333,7 @@ class NDB_BVL_Battery
                       t.isDirectEntry,
                       MAX(b.instr_order) as instrument_order
                   FROM flag f
-            	      JOIN test_names t ON (f.Test_name=t.Test_name)
+            	      JOIN test_names t ON (f.TestID=t.ID)
             	      JOIN test_subgroups ts ON (ts.ID = t.Sub_group)
             	      LEFT JOIN session s ON (s.ID=f.SessionID)
             	      LEFT JOIN test_battery b
@@ -337,7 +343,7 @@ class NDB_BVL_Battery
        	                AND (s.CenterID=b.CenterID OR b.CenterID IS NULL))
        	          WHERE f.SessionID=:SID
                       AND LEFT(f.CommentID, 4) != 'DDE_'
-                  GROUP BY f.Test_name,
+                  GROUP BY t.Test_name,
                            t.Full_name,
                            f.CommentID,
                            DDECommentID,
@@ -391,7 +397,9 @@ class NDB_BVL_Battery
     {
         return \NDB_Factory::singleton()->database()
             ->pselectOne(
-                "SELECT Test_name FROM flag WHERE CommentID=:CID",
+                "SELECT Test_name FROM flag
+		    	JOIN test_names ON (flag.TestID=test_names.ID)
+		    WHERE CommentID=:CID",
                 ['CID' => $commentId]
             );
     }

diff --git a/php/libraries/NDB_BVL_Feedback.class.inc b/php/libraries/NDB_BVL_Feedback.class.inc
@@ -465,14 +465,15 @@ class NDB_BVL_Feedback
                     ft.Feedback_level as QC_Class,
                     COUNT(ft.FeedbackID) as No_Threads";
         if (empty($this->_feedbackObjectInfo['CandID'])) {
-            $query .= ", f.Test_name as Instrument, ft.CommentID";
+            $query .= ", tn.Test_name as Instrument, ft.CommentID";
         }
         $query  .= " FROM candidate as c
                         JOIN feedback_bvl_thread as ft ON (c.CandID=ft.CandID)
                         LEFT JOIN session as s ON (s.ID = ft.SessionID)";
         $qparams = [];
         if (empty($this->_feedbackObjectInfo['CandID'])) {
-            $query .= " LEFT JOIN flag as f ON (ft.CommentID = f.CommentID)";
+            $query .= " LEFT JOIN flag as f ON (ft.CommentID = f.CommentID)
+                             JOIN test_names tn ON (f.TestID=tn.ID) ";
         }
         $query .= " WHERE ft.Active ='Y'";
 
@@ -551,7 +552,7 @@ class NDB_BVL_Feedback
                      ft.Date_taken as Date,
                      DATE_FORMAT(ft.Testdate, '%Y-%m-%d') as Modified";
         if (!empty($this->_feedbackObjectInfo['CommentID'])) {
-            $query .= ", f.Test_name as Instrument, f.CommentID";
+            $query .= ", tn.Test_name as Instrument, f.CommentID";
         }
 
         $query .= " FROM candidate as c
@@ -562,7 +563,7 @@ class NDB_BVL_Feedback
                      JOIN feedback_bvl_type as ftp
                          ON (ftp.Feedback_type=ft.Feedback_type)";
         if (!empty($this->_feedbackObjectInfo['CommentID'])) {
-            $query .= ", flag as f";
+            $query .= ", flag as f LEFT JOIN test_names tn ON (tn.ID=f.TestID)";
         }
 
         // FIXME: This clause is kept here so that the ANDs below don't
@@ -622,7 +623,7 @@ class NDB_BVL_Feedback
                              ft.Status,
                              ft.Testdate DESC";
 
-            $result = $db->pselect($query, $qparams);
+        $result = $db->pselect($query, $qparams);
 
         return iterator_to_array($result);
     }

diff --git a/php/libraries/NDB_BVL_Instrument.class.inc b/php/libraries/NDB_BVL_Instrument.class.inc
@@ -637,7 +637,8 @@ abstract class NDB_BVL_Instrument extends NDB_Page
                 $validage = $DB->pselectOne(
                     "SELECT MAX($agedays BETWEEN AgeMinDays AND AgeMaxDays)
                         FROM test_battery tb
-                            JOIN flag f USING(Test_name)
+                            JOIN test_names tn USING(Test_name)
+                            JOIN flag f ON (f.TestID=tn.ID)
                             JOIN session s ON (s.ID=f.SessionID)
                         WHERE tb.Active='Y'
                             AND tb.Test_name=:TN
@@ -654,7 +655,8 @@ abstract class NDB_BVL_Instrument extends NDB_Page
                     $Windows = $DB->pselect(
                         "SELECT AgeMinDays, AgeMaxDays
                         FROM test_battery tb
-                            JOIN flag f USING(Test_name)
+                            JOIN test_names tn USING(Test_name)
+                            JOIN flag f ON (f.TestID=tn.ID)
                             JOIN session s ON (s.ID=f.SessionID)
                         WHERE tb.Active='Y' AND tb.Test_name=:TN
                             AND tb.CohortID=s.CohortID

diff --git a/php/libraries/TimePoint.class.inc b/php/libraries/TimePoint.class.inc
@@ -1185,11 +1185,12 @@ class TimePoint implements \LORIS\StudyEntities\AccessibleResource,
         $result = $db->pselectCol(
             '
              SELECT DISTINCT
-               Test_name as shortname
+               tn.Test_name as shortname
              FROM
-               flag
+               flag f
+             JOIN test_names tn ON (f.TestID=tn.ID)
              WHERE
-               SessionID = :v_sessionid
+               f.SessionID = :v_sessionid
             ',
             ['v_sessionid' => $this->getSessionID()]
         );