changeset 415:7e123f047771

LookupResources optimization continued
author Alain Mazy <am@osimis.io>
date Thu, 22 Jun 2023 18:07:41 +0200
parents f2d3b5c5a68d
children 6b16914b6655
files Framework/Plugins/IndexBackend.cpp MySQL/NEWS Odbc/NEWS PostgreSQL/NEWS Resources/Orthanc/Databases/ISqlLookupFormatter.cpp Resources/Orthanc/Databases/ISqlLookupFormatter.h
diffstat 6 files changed, 110 insertions(+), 79 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/Plugins/IndexBackend.cpp	Thu Jun 22 12:15:48 2023 +0200
+++ b/Framework/Plugins/IndexBackend.cpp	Thu Jun 22 18:07:41 2023 +0200
@@ -2063,46 +2063,79 @@
   void IndexBackend::LookupResources(IDatabaseBackendOutput& output,
                                      DatabaseManager& manager,
                                      const std::vector<Orthanc::DatabaseConstraint>& lookup,
-                                     OrthancPluginResourceType queryLevel,
+                                     OrthancPluginResourceType queryLevel_,
                                      const std::set<std::string>& labels,
                                      Orthanc::LabelsConstraint labelsConstraint,
                                      uint32_t limit,
                                      bool requestSomeInstance)
   {
     LookupFormatter formatter(manager.GetDialect());
+    Orthanc::ResourceType queryLevel = Orthanc::Plugins::Convert(queryLevel_);
+    Orthanc::ResourceType lowerLevel, upperLevel;
+    Orthanc::ISqlLookupFormatter::GetLookupLevels(lowerLevel, upperLevel,  queryLevel, lookup);
 
     std::string sql;
     bool enableNewStudyCode = true;
 
-    if (enableNewStudyCode && queryLevel == OrthancPluginResourceType_Study)
+    if (enableNewStudyCode && lowerLevel == queryLevel && upperLevel == queryLevel)
     {
-      // separate path for the studies since it has been specifically optimized
-      Orthanc::ISqlLookupFormatter::ApplyExperimental(sql, formatter, lookup, Orthanc::Plugins::Convert(queryLevel), labels, labelsConstraint, limit);
-
-      if (requestSomeInstance)
-      {
-        sql = ("SELECT studies_series.studies_public_id, MIN(instances.publicId) AS instances_public_id "
-                "FROM (SELECT studies.publicId AS studies_public_id, MIN(series.internalId) AS series_internal_id "
-                      "FROM (" + sql + 
-                            ") AS studies "
-                            "INNER JOIN Resources series ON series.parentId = studies.internalId "
-                            "GROUP BY studies.publicId "
-                      ") AS studies_series "
-                "INNER JOIN Resources instances ON instances.parentId = studies_series.series_internal_id "
-                "GROUP BY studies_series.studies_public_id");
-      }
-    }
-    else
-    {
-      Orthanc::ISqlLookupFormatter::Apply(sql, formatter, lookup, Orthanc::Plugins::Convert(queryLevel),
-                                          labels, labelsConstraint, limit);      
+      Orthanc::ISqlLookupFormatter::ApplySingleLevel(sql, formatter, lookup, queryLevel, labels, labelsConstraint, limit);
 
       if (requestSomeInstance)
       {
         // Composite query to find some instance if requested
         switch (queryLevel)
         {
-          case OrthancPluginResourceType_Patient:
+          case Orthanc::ResourceType_Patient:
+            sql = ("SELECT patients_studies.patients_public_id, MIN(instances.publicId) AS instances_public_id "
+                    "FROM (SELECT patients.publicId AS patients_public_id, MIN(studies.internalId) AS studies_internal_id "
+                          "FROM (" + sql + 
+                                ") AS patients "
+                                "INNER JOIN Resources studies ON studies.parentId = patients.internalId "
+                                "GROUP BY patients.publicId "
+                          ") AS patients_studies "
+                    "INNER JOIN Resources series ON series.parentId = patients_studies.studies_internal_id "
+                    "INNER JOIN Resources instances ON instances.parentId = series.internalId "
+                    "GROUP BY patients_studies.patients_public_id");
+            break;
+          case Orthanc::ResourceType_Study:
+            sql = ("SELECT studies_series.studies_public_id, MIN(instances.publicId) AS instances_public_id "
+                    "FROM (SELECT studies.publicId AS studies_public_id, MIN(series.internalId) AS series_internal_id "
+                          "FROM (" + sql + 
+                                ") AS studies "
+                                "INNER JOIN Resources series ON series.parentId = studies.internalId "
+                                "GROUP BY studies.publicId "
+                          ") AS studies_series "
+                    "INNER JOIN Resources instances ON instances.parentId = studies_series.series_internal_id "
+                    "GROUP BY studies_series.studies_public_id");
+            break;
+          case Orthanc::ResourceType_Series:
+            sql = ("SELECT series.publicId AS series_public_id, MIN(instances.publicId) AS instances_public_id "
+                          "FROM (" + sql + 
+                                ") AS series "
+                                "INNER JOIN Resources instances ON instances.parentId = series.internalId "
+                                "GROUP BY series.publicId ");
+            break;
+
+          case Orthanc::ResourceType_Instance:
+            sql = ("SELECT instances.publicId, instances.publicId FROM (" + sql + ") instances");
+            break;
+
+          default:
+            throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError);
+        }
+      }
+    }
+    else
+    {
+      Orthanc::ISqlLookupFormatter::Apply(sql, formatter, lookup, queryLevel, labels, labelsConstraint, limit);      
+
+      if (requestSomeInstance)
+      {
+        // Composite query to find some instance if requested
+        switch (queryLevel)
+        {
+          case Orthanc::ResourceType_Patient:
             sql = ("SELECT patients.publicId, MIN(instances.publicId) FROM (" + sql + ") patients "
                   "INNER JOIN Resources studies   ON studies.parentId   = patients.internalId "
                   "INNER JOIN Resources series    ON series.parentId    = studies.internalId "
@@ -2110,19 +2143,19 @@
                   "GROUP BY patients.publicId");
             break;
 
-          case OrthancPluginResourceType_Study:
+          case Orthanc::ResourceType_Study:
             sql = ("SELECT studies.publicId, MIN(instances.publicId) FROM (" + sql + ") studies "
                   "INNER JOIN Resources series    ON series.parentId    = studies.internalId "
                   "INNER JOIN Resources instances ON instances.parentId = series.internalId "
                   "GROUP BY studies.publicId");                 
             break;
-          case OrthancPluginResourceType_Series:
+          case Orthanc::ResourceType_Series:
             sql = ("SELECT series.publicId, MIN(instances.publicId) FROM (" + sql + ") series "
                   "INNER JOIN Resources instances ON instances.parentId = series.internalId "
                   "GROUP BY series.publicId");
             break;
 
-          case OrthancPluginResourceType_Instance:
+          case Orthanc::ResourceType_Instance:
             sql = ("SELECT instances.publicId, instances.publicId FROM (" + sql + ") instances");
             break;
 
--- a/MySQL/NEWS	Thu Jun 22 12:15:48 2023 +0200
+++ b/MySQL/NEWS	Thu Jun 22 18:07:41 2023 +0200
@@ -1,6 +1,9 @@
 Pending changes in the mainline
 ===============================
 
+* Optimization of LookupResources mainly used in tools/find, C-Find and QIDO-RS.
+  The optimization mainly affects find at study level.
+
 
 Release 5.0 (2023-04-16)
 ========================
--- a/Odbc/NEWS	Thu Jun 22 12:15:48 2023 +0200
+++ b/Odbc/NEWS	Thu Jun 22 18:07:41 2023 +0200
@@ -1,6 +1,8 @@
 Pending changes in the mainline
 ===============================
 
+* Optimization of LookupResources mainly used in tools/find, C-Find and QIDO-RS.
+  The optimization mainly affects find at study level.
 * Added support for labels
 * Compatibility with Orthanc SDK 1.12.0 (communications between the
   Orthanc core and the database plugin using Google Protocol Buffers)
--- a/PostgreSQL/NEWS	Thu Jun 22 12:15:48 2023 +0200
+++ b/PostgreSQL/NEWS	Thu Jun 22 18:07:41 2023 +0200
@@ -1,6 +1,8 @@
 Pending changes in the mainline
 ===============================
 
+* Optimization of LookupResources mainly used in tools/find, C-Find and QIDO-RS.
+  The optimization mainly affects find at study level.
 
 Release 5.0 (2023-04-15)
 ========================
--- a/Resources/Orthanc/Databases/ISqlLookupFormatter.cpp	Thu Jun 22 12:15:48 2023 +0200
+++ b/Resources/Orthanc/Databases/ISqlLookupFormatter.cpp	Thu Jun 22 18:07:41 2023 +0200
@@ -475,23 +475,14 @@
   }
 
 
-
-  
-
-  void ISqlLookupFormatter::Apply(std::string& sql,
-                                  ISqlLookupFormatter& formatter,
-                                  const std::vector<DatabaseConstraint>& lookup,
-                                  ResourceType queryLevel,
-                                  const std::set<std::string>& labels,
-                                  LabelsConstraint labelsConstraint,
-                                  size_t limit)
+  void ISqlLookupFormatter::GetLookupLevels(ResourceType& lowerLevel, ResourceType& upperLevel, const ResourceType& queryLevel, const std::vector<DatabaseConstraint>& lookup)
   {
     assert(ResourceType_Patient < ResourceType_Study &&
            ResourceType_Study < ResourceType_Series &&
            ResourceType_Series < ResourceType_Instance);
     
-    ResourceType upperLevel = queryLevel;
-    ResourceType lowerLevel = queryLevel;
+    lowerLevel = queryLevel;
+    upperLevel = queryLevel;
 
     for (size_t i = 0; i < lookup.size(); i++)
     {
@@ -507,7 +498,20 @@
         lowerLevel = level;
       }
     }
-    
+  }
+  
+
+  void ISqlLookupFormatter::Apply(std::string& sql,
+                                  ISqlLookupFormatter& formatter,
+                                  const std::vector<DatabaseConstraint>& lookup,
+                                  ResourceType queryLevel,
+                                  const std::set<std::string>& labels,
+                                  LabelsConstraint labelsConstraint,
+                                  size_t limit)
+  {
+    ResourceType lowerLevel, upperLevel;
+    GetLookupLevels(lowerLevel, upperLevel, queryLevel, lookup);
+
     assert(upperLevel <= queryLevel &&
            queryLevel <= lowerLevel);
 
@@ -608,39 +612,20 @@
   }
 
 
-  void ISqlLookupFormatter::ApplyExperimental(std::string& sql,
-                                              ISqlLookupFormatter& formatter,
-                                              const std::vector<DatabaseConstraint>& lookup,
-                                              ResourceType queryLevel,
-                                              const std::set<std::string>& labels,
-                                              LabelsConstraint labelsConstraint,
-                                              size_t limit
-                                              )
+  void ISqlLookupFormatter::ApplySingleLevel(std::string& sql,
+                                             ISqlLookupFormatter& formatter,
+                                             const std::vector<DatabaseConstraint>& lookup,
+                                             ResourceType queryLevel,
+                                             const std::set<std::string>& labels,
+                                             LabelsConstraint labelsConstraint,
+                                             size_t limit
+                                             )
   {
-    assert(ResourceType_Patient < ResourceType_Study &&
-           ResourceType_Study < ResourceType_Series &&
-           ResourceType_Series < ResourceType_Instance);
+    ResourceType lowerLevel, upperLevel;
+    GetLookupLevels(lowerLevel, upperLevel, queryLevel, lookup);
     
-    ResourceType upperLevel = queryLevel;
-    ResourceType lowerLevel = queryLevel;
-
-    for (size_t i = 0; i < lookup.size(); i++)
-    {
-      ResourceType level = lookup[i].GetLevel();
-
-      if (level < upperLevel)
-      {
-        upperLevel = level;
-      }
-
-      if (level > lowerLevel)
-      {
-        lowerLevel = level;
-      }
-    }
-    
-    assert(upperLevel <= queryLevel &&
-           queryLevel <= lowerLevel);
+    assert(upperLevel == queryLevel &&
+           queryLevel == lowerLevel);
 
     const bool escapeBrackets = formatter.IsEscapeBrackets();
     
@@ -703,25 +688,29 @@
       }
 
       std::string condition;
+      std::string inOrNotIn;
       switch (labelsConstraint)
       {
         case LabelsConstraint_Any:
           condition = "> 0";
+          inOrNotIn = "IN";
           break;
           
         case LabelsConstraint_All:
           condition = "= " + boost::lexical_cast<std::string>(labels.size());
+          inOrNotIn = "IN";
           break;
           
         case LabelsConstraint_None:
-          condition = "= 0";
+          condition = "> 0";
+          inOrNotIn = "NOT IN";
           break;
           
         default:
           throw OrthancException(ErrorCode_ParameterOutOfRange);
       }
       
-      sql += (" AND internalId IN (SELECT id"
+      sql += (" AND internalId " + inOrNotIn + " (SELECT id"
                                  " FROM (SELECT id, COUNT(1) AS labelsCount "
                                         "FROM Labels "
                                         "WHERE label IN (" + Join(formattedLabels, "", ", ") + ") GROUP BY id"
--- a/Resources/Orthanc/Databases/ISqlLookupFormatter.h	Thu Jun 22 12:15:48 2023 +0200
+++ b/Resources/Orthanc/Databases/ISqlLookupFormatter.h	Thu Jun 22 18:07:41 2023 +0200
@@ -63,6 +63,8 @@
      **/
     virtual bool IsEscapeBrackets() const = 0;
 
+    static void GetLookupLevels(ResourceType& lowerLevel, ResourceType& upperLevel, const ResourceType& queryLevel, const std::vector<DatabaseConstraint>& lookup);
+
     static void Apply(std::string& sql,
                       ISqlLookupFormatter& formatter,
                       const std::vector<DatabaseConstraint>& lookup,
@@ -71,12 +73,12 @@
                       LabelsConstraint labelsConstraint,    // New in Orthanc 1.12.0
                       size_t limit);
 
-    static void ApplyExperimental(std::string& sql,
-                                  ISqlLookupFormatter& formatter,
-                                  const std::vector<DatabaseConstraint>& lookup,
-                                  ResourceType queryLevel,
-                                  const std::set<std::string>& labels,  // New in Orthanc 1.12.0
-                                  LabelsConstraint labelsConstraint,    // New in Orthanc 1.12.0
-                                  size_t limit);
+    static void ApplySingleLevel(std::string& sql,
+                                 ISqlLookupFormatter& formatter,
+                                 const std::vector<DatabaseConstraint>& lookup,
+                                 ResourceType queryLevel,
+                                 const std::set<std::string>& labels,  // New in Orthanc 1.12.0
+                                 LabelsConstraint labelsConstraint,    // New in Orthanc 1.12.0
+                                 size_t limit);
   };
 }