Mercurial > hg > orthanc-databases
changeset 536:4ecf50a4521c find-refactoring
sync ISqlLookupFormatter from Orthanc + fix bug 224: LIMIT shall not be used with MSSQL
| author | Alain Mazy <am@orthanc.team> |
|---|---|
| date | Fri, 06 Sep 2024 16:56:37 +0200 |
| parents | 03a4a1bc852a |
| children | a8bf2d6e86ef |
| files | Framework/Plugins/DatabaseBackendAdapterV2.cpp Framework/Plugins/DatabaseBackendAdapterV3.cpp Framework/Plugins/DatabaseBackendAdapterV4.cpp Framework/Plugins/IDatabaseBackend.h Framework/Plugins/IndexBackend.cpp Framework/Plugins/IndexBackend.h Odbc/NEWS Resources/Orthanc/CMake/Compiler.cmake Resources/Orthanc/Databases/DatabaseConstraint.cpp Resources/Orthanc/Databases/DatabaseConstraint.h Resources/Orthanc/Databases/ISqlLookupFormatter.cpp Resources/Orthanc/Databases/ISqlLookupFormatter.h |
| diffstat | 12 files changed, 377 insertions(+), 31 deletions(-) [+] |
line wrap: on
line diff
--- a/Framework/Plugins/DatabaseBackendAdapterV2.cpp Fri Sep 06 15:44:40 2024 +0200 +++ b/Framework/Plugins/DatabaseBackendAdapterV2.cpp Fri Sep 06 16:56:37 2024 +0200 @@ -1413,12 +1413,11 @@ { DatabaseBackendAdapterV2::Adapter::DatabaseAccessor accessor(*adapter); - std::vector<Orthanc::DatabaseConstraint> lookup; - lookup.reserve(constraintsCount); + Orthanc::DatabaseConstraints lookup; for (uint32_t i = 0; i < constraintsCount; i++) { - lookup.push_back(Orthanc::DatabaseConstraint(constraints[i])); + lookup.AddConstraint(new Orthanc::DatabaseConstraint(constraints[i])); } std::set<std::string> noLabel;
--- a/Framework/Plugins/DatabaseBackendAdapterV3.cpp Fri Sep 06 15:44:40 2024 +0200 +++ b/Framework/Plugins/DatabaseBackendAdapterV3.cpp Fri Sep 06 16:56:37 2024 +0200 @@ -1640,12 +1640,11 @@ { t->GetOutput().Clear(); - std::vector<Orthanc::DatabaseConstraint> lookup; - lookup.reserve(constraintsCount); + Orthanc::DatabaseConstraints lookup; for (uint32_t i = 0; i < constraintsCount; i++) { - lookup.push_back(Orthanc::DatabaseConstraint(constraints[i])); + lookup.AddConstraint(new Orthanc::DatabaseConstraint(constraints[i])); } std::set<std::string> noLabel;
--- a/Framework/Plugins/DatabaseBackendAdapterV4.cpp Fri Sep 06 15:44:40 2024 +0200 +++ b/Framework/Plugins/DatabaseBackendAdapterV4.cpp Fri Sep 06 16:56:37 2024 +0200 @@ -119,6 +119,7 @@ Orthanc::DatabasePluginMessages::DeleteAttachment::Response* deleteAttachment_; Orthanc::DatabasePluginMessages::DeleteResource::Response* deleteResource_; Orthanc::DatabasePluginMessages::GetChanges::Response* getChanges_; + Orthanc::DatabasePluginMessages::GetChangesExtended::Response* getChangesExtended_; Orthanc::DatabasePluginMessages::GetExportedResources::Response* getExportedResources_; Orthanc::DatabasePluginMessages::GetLastChange::Response* getLastChange_; Orthanc::DatabasePluginMessages::GetLastExportedResource::Response* getLastExportedResource_; @@ -131,6 +132,7 @@ deleteAttachment_ = NULL; deleteResource_ = NULL; getChanges_ = NULL; + getChangesExtended_ = NULL; getExportedResources_ = NULL; getLastChange_ = NULL; getLastExportedResource_ = NULL; @@ -157,7 +159,13 @@ Clear(); getChanges_ = &getChanges; } - + + Output(Orthanc::DatabasePluginMessages::GetChangesExtended::Response& getChangesExtended) + { + Clear(); + getChangesExtended_ = &getChangesExtended; + } + Output(Orthanc::DatabasePluginMessages::GetExportedResources::Response& getExportedResources) { Clear(); @@ -310,6 +318,10 @@ { change = getChanges_->add_changes(); } + else if (getChangesExtended_ != NULL) + { + change = getChangesExtended_->add_changes(); + } else if (getLastChange_ != NULL) { if (getLastChange_->found()) @@ -549,8 +561,7 @@ IndexBackend& backend, DatabaseManager& manager) { - std::vector<Orthanc::DatabaseConstraint> lookup; - lookup.reserve(request.lookup().size()); + Orthanc::DatabaseConstraints lookup; size_t countValues = 0; @@ -624,7 +635,7 @@ } } - lookup.push_back(Orthanc::DatabaseConstraint(c)); + lookup.AddConstraint(new Orthanc::DatabaseConstraint(c)); } assert(values.size() == countValues); @@ -791,7 +802,7 @@ #if ORTHANC_PLUGINS_VERSION_IS_ABOVE(1, 12, 5) case Orthanc::DatabasePluginMessages::OPERATION_GET_CHANGES_EXTENDED: { - Output output(*response.mutable_get_changes()); + Output output(*response.mutable_get_changes_extended()); bool done; backend.GetChangesExtended(output, done, manager, request.get_changes_extended().since(), request.get_changes_extended().to(), static_cast<OrthancPluginChangeType>(request.get_changes_extended().change_type()), request.get_changes_extended().limit());
--- a/Framework/Plugins/IDatabaseBackend.h Fri Sep 06 15:44:40 2024 +0200 +++ b/Framework/Plugins/IDatabaseBackend.h Fri Sep 06 16:56:37 2024 +0200 @@ -293,7 +293,7 @@ #if ORTHANC_PLUGINS_HAS_DATABASE_CONSTRAINT == 1 virtual void LookupResources(IDatabaseBackendOutput& output, DatabaseManager& manager, - const std::vector<Orthanc::DatabaseConstraint>& lookup, + Orthanc::DatabaseConstraints& lookup, OrthancPluginResourceType queryLevel, const std::set<std::string>& labels, // New in Orthanc 1.12.0 Orthanc::LabelsConstraint labelsConstraint, // New in Orthanc 1.12.0
--- a/Framework/Plugins/IndexBackend.cpp Fri Sep 06 15:44:40 2024 +0200 +++ b/Framework/Plugins/IndexBackend.cpp Fri Sep 06 16:56:37 2024 +0200 @@ -2178,6 +2178,43 @@ } } + virtual std::string FormatLimits(uint64_t since, uint64_t count) + { + std::string sql; + + switch (dialect_) + { + case Dialect_MSSQL: + { + if (since > 0) + { + sql += " OFFSET " + boost::lexical_cast<std::string>(since) + " ROWS "; + } + if (count > 0) + { + sql += " FETCH NEXT " + boost::lexical_cast<std::string>(count) + " ROWS ONLY "; + } + }; break; + case Dialect_SQLite: + case Dialect_PostgreSQL: + case Dialect_MySQL: + { + if (count > 0) + { + sql += " LIMIT " + boost::lexical_cast<std::string>(count); + } + if (since > 0) + { + sql += " OFFSET " + boost::lexical_cast<std::string>(since); + } + }; break; + default: + throw Orthanc::OrthancException(Orthanc::ErrorCode_NotImplemented); + } + + return sql; + } + virtual bool IsEscapeBrackets() const { // This was initially done at a bad location by the following changeset: @@ -2207,7 +2244,7 @@ // New primitive since Orthanc 1.5.2 void IndexBackend::LookupResources(IDatabaseBackendOutput& output, DatabaseManager& manager, - const std::vector<Orthanc::DatabaseConstraint>& lookup, + Orthanc::DatabaseConstraints& lookup, OrthancPluginResourceType queryLevel_, const std::set<std::string>& labels, Orthanc::LabelsConstraint labelsConstraint,
--- a/Framework/Plugins/IndexBackend.h Fri Sep 06 15:44:40 2024 +0200 +++ b/Framework/Plugins/IndexBackend.h Fri Sep 06 16:56:37 2024 +0200 @@ -312,7 +312,7 @@ // New primitive since Orthanc 1.5.2 virtual void LookupResources(IDatabaseBackendOutput& output, DatabaseManager& manager, - const std::vector<Orthanc::DatabaseConstraint>& lookup, + Orthanc::DatabaseConstraints& lookup, OrthancPluginResourceType queryLevel, const std::set<std::string>& labels, Orthanc::LabelsConstraint labelsConstraint,
--- a/Odbc/NEWS Fri Sep 06 15:44:40 2024 +0200 +++ b/Odbc/NEWS Fri Sep 06 16:56:37 2024 +0200 @@ -9,6 +9,8 @@ * Fix check of Orthanc runtime version * Added support for ExtendedChanges: - changes?type=...&to=... +* Fix bug 224, error when using LIMIT with MSSQLServer + https://orthanc.uclouvain.be/bugs/show_bug.cgi?id=224 Release 1.2 (2024-03-06)
--- a/Resources/Orthanc/CMake/Compiler.cmake Fri Sep 06 15:44:40 2024 +0200 +++ b/Resources/Orthanc/CMake/Compiler.cmake Fri Sep 06 16:56:37 2024 +0200 @@ -232,6 +232,10 @@ endif() elseif (${CMAKE_SYSTEM_NAME} STREQUAL "Darwin") + + # fix this error that appears with recent compilers on MacOS: boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for this enumeration type [-Wenum-constexpr-conversion] + SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-enum-constexpr-conversion") + add_definitions( -D_XOPEN_SOURCE=1 )
--- a/Resources/Orthanc/Databases/DatabaseConstraint.cpp Fri Sep 06 15:44:40 2024 +0200 +++ b/Resources/Orthanc/Databases/DatabaseConstraint.cpp Fri Sep 06 16:56:37 2024 +0200 @@ -37,6 +37,7 @@ # include <OrthancException.h> #endif +#include <boost/lexical_cast.hpp> #include <cassert> @@ -245,4 +246,103 @@ constraint.values = (tmpValues.empty() ? NULL : &tmpValues[0]); } #endif + + + void DatabaseConstraints::Clear() + { + for (size_t i = 0; i < constraints_.size(); i++) + { + assert(constraints_[i] != NULL); + delete constraints_[i]; + } + + constraints_.clear(); + } + + + void DatabaseConstraints::AddConstraint(DatabaseConstraint* constraint) + { + if (constraint == NULL) + { + throw OrthancException(ErrorCode_NullPointer); + } + else + { + constraints_.push_back(constraint); + } + } + + + const DatabaseConstraint& DatabaseConstraints::GetConstraint(size_t index) const + { + if (index >= constraints_.size()) + { + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + else + { + assert(constraints_[index] != NULL); + return *constraints_[index]; + } + } + + + std::string DatabaseConstraints::Format() const + { + std::string s; + + for (size_t i = 0; i < constraints_.size(); i++) + { + assert(constraints_[i] != NULL); + const DatabaseConstraint& constraint = *constraints_[i]; + s += "Constraint " + boost::lexical_cast<std::string>(i) + " at " + EnumerationToString(constraint.GetLevel()) + + ": " + constraint.GetTag().Format(); + + switch (constraint.GetConstraintType()) + { + case ConstraintType_Equal: + s += " == " + constraint.GetSingleValue(); + break; + + case ConstraintType_SmallerOrEqual: + s += " <= " + constraint.GetSingleValue(); + break; + + case ConstraintType_GreaterOrEqual: + s += " >= " + constraint.GetSingleValue(); + break; + + case ConstraintType_Wildcard: + s += " ~~ " + constraint.GetSingleValue(); + break; + + case ConstraintType_List: + { + s += " in [ "; + bool first = true; + for (size_t j = 0; j < constraint.GetValuesCount(); j++) + { + if (first) + { + first = false; + } + else + { + s += ", "; + } + s += constraint.GetValue(j); + } + s += "]"; + break; + } + + default: + throw OrthancException(ErrorCode_InternalError); + } + + s += "\n"; + } + + return s; + } }
--- a/Resources/Orthanc/Databases/DatabaseConstraint.h Fri Sep 06 15:44:40 2024 +0200 +++ b/Resources/Orthanc/Databases/DatabaseConstraint.h Fri Sep 06 16:56:37 2024 +0200 @@ -46,6 +46,8 @@ # endif #endif +#include <deque> + namespace Orthanc { enum ConstraintType @@ -78,7 +80,7 @@ // This class is also used by the "orthanc-databases" project - class DatabaseConstraint + class DatabaseConstraint : public boost::noncopyable { private: ResourceType level_; @@ -148,4 +150,35 @@ std::vector<const char*>& tmpValues) const; #endif }; + + + class DatabaseConstraints : public boost::noncopyable + { + private: + std::deque<DatabaseConstraint*> constraints_; + + public: + ~DatabaseConstraints() + { + Clear(); + } + + void Clear(); + + void AddConstraint(DatabaseConstraint* constraint); // Takes ownership + + bool IsEmpty() const + { + return constraints_.empty(); + } + + size_t GetSize() const + { + return constraints_.size(); + } + + const DatabaseConstraint& GetConstraint(size_t index) const; + + std::string Format() const; + }; }
--- a/Resources/Orthanc/Databases/ISqlLookupFormatter.cpp Fri Sep 06 15:44:40 2024 +0200 +++ b/Resources/Orthanc/Databases/ISqlLookupFormatter.cpp Fri Sep 06 16:56:37 2024 +0200 @@ -34,6 +34,7 @@ #if ORTHANC_BUILDING_SERVER_LIBRARY == 1 # include "../../../OrthancFramework/Sources/OrthancException.h" # include "../../../OrthancFramework/Sources/Toolbox.h" +# include "../Database/FindRequest.h" #else # include <OrthancException.h> # include <Toolbox.h> @@ -475,7 +476,10 @@ } - void ISqlLookupFormatter::GetLookupLevels(ResourceType& lowerLevel, ResourceType& upperLevel, const ResourceType& queryLevel, const std::vector<DatabaseConstraint>& lookup) + void ISqlLookupFormatter::GetLookupLevels(ResourceType& lowerLevel, + ResourceType& upperLevel, + const ResourceType& queryLevel, + const DatabaseConstraints& lookup) { assert(ResourceType_Patient < ResourceType_Study && ResourceType_Study < ResourceType_Series && @@ -484,9 +488,9 @@ lowerLevel = queryLevel; upperLevel = queryLevel; - for (size_t i = 0; i < lookup.size(); i++) + for (size_t i = 0; i < lookup.GetSize(); i++) { - ResourceType level = lookup[i].GetLevel(); + ResourceType level = lookup.GetConstraint(i).GetLevel(); if (level < upperLevel) { @@ -503,7 +507,7 @@ void ISqlLookupFormatter::Apply(std::string& sql, ISqlLookupFormatter& formatter, - const std::vector<DatabaseConstraint>& lookup, + const DatabaseConstraints& lookup, ResourceType queryLevel, const std::set<std::string>& labels, LabelsConstraint labelsConstraint, @@ -521,14 +525,16 @@ size_t count = 0; - for (size_t i = 0; i < lookup.size(); i++) + for (size_t i = 0; i < lookup.GetSize(); i++) { + const DatabaseConstraint& constraint = lookup.GetConstraint(i); + std::string comparison; - if (FormatComparison(comparison, formatter, lookup[i], count, escapeBrackets)) + if (FormatComparison(comparison, formatter, constraint, count, escapeBrackets)) { std::string join; - FormatJoin(join, lookup[i], count); + FormatJoin(join, constraint, count); joins += join; if (!comparison.empty()) @@ -611,10 +617,151 @@ } } +#if ORTHANC_BUILDING_SERVER_LIBRARY == 1 + void ISqlLookupFormatter::Apply(std::string& sql, + ISqlLookupFormatter& formatter, + const FindRequest& request) + { + const bool escapeBrackets = formatter.IsEscapeBrackets(); + ResourceType queryLevel = request.GetLevel(); + const std::string& strQueryLevel = FormatLevel(queryLevel); + + ResourceType lowerLevel, upperLevel; + GetLookupLevels(lowerLevel, upperLevel, queryLevel, request.GetDicomTagConstraints()); + + assert(upperLevel <= queryLevel && + queryLevel <= lowerLevel); + + + sql = ("SELECT " + + strQueryLevel + ".publicId, " + + strQueryLevel + ".internalId" + + " FROM Resources AS " + strQueryLevel); + + + std::string joins, comparisons; + + if (request.GetOrthancIdentifiers().IsDefined() && request.GetOrthancIdentifiers().DetectLevel() <= queryLevel) + { + // single child resource matching, there should not be other constraints (at least for now) + assert(request.GetDicomTagConstraints().GetSize() == 0); + assert(request.GetLabels().size() == 0); + assert(request.HasLimits() == false); + + ResourceType topParentLevel = request.GetOrthancIdentifiers().DetectLevel(); + const std::string& strTopParentLevel = FormatLevel(topParentLevel); + + comparisons = " AND " + strTopParentLevel + ".publicId = " + formatter.GenerateParameter(request.GetOrthancIdentifiers().GetLevel(topParentLevel)); + + for (int level = queryLevel; level > topParentLevel; level--) + { + sql += (" INNER JOIN Resources " + + FormatLevel(static_cast<ResourceType>(level - 1)) + " ON " + + FormatLevel(static_cast<ResourceType>(level - 1)) + ".internalId=" + + FormatLevel(static_cast<ResourceType>(level)) + ".parentId"); + } + } + else + { + size_t count = 0; + + const DatabaseConstraints& dicomTagsConstraints = request.GetDicomTagConstraints(); + for (size_t i = 0; i < dicomTagsConstraints.GetSize(); i++) + { + const DatabaseConstraint& constraint = dicomTagsConstraints.GetConstraint(i); + + std::string comparison; + + if (FormatComparison(comparison, formatter, constraint, count, escapeBrackets)) + { + std::string join; + FormatJoin(join, constraint, count); + joins += join; + + if (!comparison.empty()) + { + comparisons += " AND " + comparison; + } + + count ++; + } + } + } + + for (int level = queryLevel - 1; level >= upperLevel; level--) + { + sql += (" INNER JOIN Resources " + + FormatLevel(static_cast<ResourceType>(level)) + " ON " + + FormatLevel(static_cast<ResourceType>(level)) + ".internalId=" + + FormatLevel(static_cast<ResourceType>(level + 1)) + ".parentId"); + } + + for (int level = queryLevel + 1; level <= lowerLevel; level++) + { + sql += (" INNER JOIN Resources " + + FormatLevel(static_cast<ResourceType>(level)) + " ON " + + FormatLevel(static_cast<ResourceType>(level - 1)) + ".internalId=" + + FormatLevel(static_cast<ResourceType>(level)) + ".parentId"); + } + + std::list<std::string> where; + where.push_back(strQueryLevel + ".resourceType = " + + formatter.FormatResourceType(queryLevel) + comparisons); + + + if (!request.GetLabels().empty()) + { + /** + * "In SQL Server, NOT EXISTS and NOT IN predicates are the best + * way to search for missing values, as long as both columns in + * question are NOT NULL." + * https://explainextended.com/2009/09/15/not-in-vs-not-exists-vs-left-join-is-null-sql-server/ + **/ + + const std::set<std::string>& labels = request.GetLabels(); + std::list<std::string> formattedLabels; + for (std::set<std::string>::const_iterator it = labels.begin(); it != labels.end(); ++it) + { + formattedLabels.push_back(formatter.GenerateParameter(*it)); + } + + std::string condition; + switch (request.GetLabelsConstraint()) + { + case LabelsConstraint_Any: + condition = "> 0"; + break; + + case LabelsConstraint_All: + condition = "= " + boost::lexical_cast<std::string>(labels.size()); + break; + + case LabelsConstraint_None: + condition = "= 0"; + break; + + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + + where.push_back("(SELECT COUNT(1) FROM Labels AS selectedLabels WHERE selectedLabels.id = " + strQueryLevel + + ".internalId AND selectedLabels.label IN (" + Join(formattedLabels, "", ", ") + ")) " + condition); + } + + sql += joins + Join(where, " WHERE ", " AND "); + + if (request.HasLimits()) + { + sql += formatter.FormatLimits(request.GetLimitsSince(), request.GetLimitsCount()); + } + + } +#endif + void ISqlLookupFormatter::ApplySingleLevel(std::string& sql, ISqlLookupFormatter& formatter, - const std::vector<DatabaseConstraint>& lookup, + const DatabaseConstraints& lookup, ResourceType queryLevel, const std::set<std::string>& labels, LabelsConstraint labelsConstraint, @@ -631,15 +778,17 @@ std::vector<std::string> mainDicomTagsComparisons, dicomIdentifiersComparisons; - for (size_t i = 0; i < lookup.size(); i++) + for (size_t i = 0; i < lookup.GetSize(); i++) { + const DatabaseConstraint& constraint = lookup.GetConstraint(i); + std::string comparison; - if (FormatComparison2(comparison, formatter, lookup[i], escapeBrackets)) + if (FormatComparison2(comparison, formatter, constraint, escapeBrackets)) { if (!comparison.empty()) { - if (lookup[i].IsIdentifier()) + if (constraint.IsIdentifier()) { dicomIdentifiersComparisons.push_back(comparison); }
--- a/Resources/Orthanc/Databases/ISqlLookupFormatter.h Fri Sep 06 15:44:40 2024 +0200 +++ b/Resources/Orthanc/Databases/ISqlLookupFormatter.h Fri Sep 06 16:56:37 2024 +0200 @@ -34,8 +34,9 @@ namespace Orthanc { - class DatabaseConstraint; - + class DatabaseConstraints; + class FindRequest; + enum LabelsConstraint { LabelsConstraint_All, @@ -57,6 +58,8 @@ virtual std::string FormatWildcardEscape() = 0; + virtual std::string FormatLimits(uint64_t since, uint64_t count) = 0; + /** * Whether to escape '[' and ']', which is only needed for * MSSQL. New in Orthanc 1.10.0, from the following changeset: @@ -64,11 +67,14 @@ **/ virtual bool IsEscapeBrackets() const = 0; - static void GetLookupLevels(ResourceType& lowerLevel, ResourceType& upperLevel, const ResourceType& queryLevel, const std::vector<DatabaseConstraint>& lookup); + static void GetLookupLevels(ResourceType& lowerLevel, + ResourceType& upperLevel, + const ResourceType& queryLevel, + const DatabaseConstraints& lookup); static void Apply(std::string& sql, ISqlLookupFormatter& formatter, - const std::vector<DatabaseConstraint>& lookup, + const DatabaseConstraints& lookup, ResourceType queryLevel, const std::set<std::string>& labels, // New in Orthanc 1.12.0 LabelsConstraint labelsConstraint, // New in Orthanc 1.12.0 @@ -76,10 +82,16 @@ static void ApplySingleLevel(std::string& sql, ISqlLookupFormatter& formatter, - const std::vector<DatabaseConstraint>& lookup, + const DatabaseConstraints& 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); + +#if ORTHANC_BUILDING_SERVER_LIBRARY == 1 + static void Apply(std::string& sql, + ISqlLookupFormatter& formatter, + const FindRequest& request); +#endif }; }