Mercurial > hg > orthanc-databases
changeset 652:e5051580aeac attach-custom-data
merged default -> attach-custom-data
| author | Alain Mazy <am@orthanc.team> |
|---|---|
| date | Mon, 10 Mar 2025 18:53:00 +0100 (7 weeks ago) |
| parents | 354d4c617387 (current diff) f9e43680c480 (diff) |
| children | d5c889dea585 |
| files | Framework/Common/DatabaseManager.cpp Framework/Common/DatabaseManager.h Framework/Plugins/IndexBackend.cpp Framework/Plugins/IndexBackend.h MySQL/Plugins/MySQLIndex.cpp MySQL/Plugins/MySQLIndex.h Odbc/Plugins/OdbcIndex.cpp Odbc/Plugins/OdbcIndex.h PostgreSQL/CMakeLists.txt PostgreSQL/NEWS PostgreSQL/Plugins/PostgreSQLIndex.cpp PostgreSQL/Plugins/PostgreSQLIndex.h PostgreSQL/Plugins/SQL/Downgrades/Rev4ToRev3.sql PostgreSQL/Plugins/SQL/PrepareIndex.sql PostgreSQL/Plugins/SQL/Upgrades/Rev2ToRev3.sql PostgreSQL/Plugins/SQL/Upgrades/Rev3ToRev4.sql SQLite/Plugins/SQLiteIndex.cpp SQLite/Plugins/SQLiteIndex.h |
| diffstat | 28 files changed, 567 insertions(+), 330 deletions(-) [+] |
line wrap: on
line diff
--- a/Framework/Common/DatabaseManager.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Common/DatabaseManager.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -568,11 +568,29 @@ StatementBase(manager), statementId_(statementId) { + Query::Parameters emptyParameters; + Setup(sql, emptyParameters); + } + + + DatabaseManager::CachedStatement::CachedStatement(const StatementId& statementId, + DatabaseManager& manager, + const std::string& sql, + const Query::Parameters& parametersTypes) : + StatementBase(manager), + statementId_(statementId) + { + Setup(sql, parametersTypes); + } + + void DatabaseManager::CachedStatement::Setup(const std::string& sql, + const Query::Parameters& parametersTypes) + { statement_ = GetManager().LookupCachedStatement(statementId_); if (statement_ == NULL) { - SetQuery(new Query(sql)); + SetQuery(new Query(sql, parametersTypes)); } else { @@ -639,7 +657,17 @@ const std::string& sql) : StatementBase(manager) { - SetQuery(new Query(sql)); + Query::Parameters emptyParameters; + SetQuery(new Query(sql, emptyParameters)); + } + + + DatabaseManager::StandaloneStatement::StandaloneStatement(DatabaseManager& manager, + const std::string& sql, + const Query::Parameters& parametersTypes) : + StatementBase(manager) + { + SetQuery(new Query(sql, parametersTypes)); }
--- a/Framework/Common/DatabaseManager.h Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Common/DatabaseManager.h Mon Mar 10 18:53:00 2025 +0100 @@ -214,11 +214,19 @@ StatementId statementId_; IPrecompiledStatement* statement_; + void Setup(const std::string& sql, + const Query::Parameters& parametersTypes); + public: CachedStatement(const StatementId& statementId, DatabaseManager& manager, const std::string& sql); + CachedStatement(const StatementId& statementId, + DatabaseManager& manager, + const std::string& sql, + const Query::Parameters& parametersTypes); + void Execute() { Dictionary parameters; @@ -249,6 +257,10 @@ StandaloneStatement(DatabaseManager& manager, const std::string& sql); + StandaloneStatement(DatabaseManager& manager, + const std::string& sql, + const Query::Parameters& parametersTypes); + virtual ~StandaloneStatement(); void Execute()
--- a/Framework/Common/Dictionary.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Common/Dictionary.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -155,4 +155,15 @@ return *found->second; } } + + void Dictionary::GetParametersType(Query::Parameters& target) const + { + target.clear(); + + for (Values::const_iterator it = values_.begin(); + it != values_.end(); ++it) + { + target[it->first] = it->second->GetType(); + } + } }
--- a/Framework/Common/Dictionary.h Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Common/Dictionary.h Mon Mar 10 18:53:00 2025 +0100 @@ -27,6 +27,7 @@ #include <map> #include <stdint.h> +#include "Query.h" namespace OrthancDatabases { @@ -74,5 +75,7 @@ void SetNullValue(const std::string& key); const IValue& GetValue(const std::string& key) const; + + void GetParametersType(Query::Parameters& target) const; }; }
--- a/Framework/Common/Query.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Common/Query.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -56,7 +56,7 @@ }; - void Query::Setup(const std::string& sql) + void Query::Setup(const std::string& sql, const Query::Parameters& parametersTypes) { boost::regex regex("\\$\\{(.*?)\\}"); @@ -76,7 +76,17 @@ parameter = parameter.substr(2, parameter.size() - 3); tokens_.push_back(new Token(true, parameter)); - parameters_[parameter] = ValueType_Utf8String; + + // when we already know the type of the parameter, set it directly to the right value + Query::Parameters::const_iterator itp = parametersTypes.find(parameter); + if (itp != parametersTypes.end()) + { + parameters_[parameter] = itp->second; + } + else + { + parameters_[parameter] = ValueType_Utf8String; + } last = it->second; @@ -93,7 +103,15 @@ Query::Query(const std::string& sql) : readOnly_(false) { - Setup(sql); + Query::Parameters emptyParameters; + Setup(sql, emptyParameters); + } + + Query::Query(const std::string& sql, + const Query::Parameters& parametersTypes) : + readOnly_(false) + { + Setup(sql, parametersTypes); } @@ -101,7 +119,16 @@ bool readOnly) : readOnly_(readOnly) { - Setup(sql); + Query::Parameters emptyParameters; + Setup(sql, emptyParameters); + } + + Query::Query(const std::string& sql, + bool readOnly, + const Query::Parameters& parametersTypes) : + readOnly_(readOnly) + { + Setup(sql, parametersTypes); }
--- a/Framework/Common/Query.h Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Common/Query.h Mon Mar 10 18:53:00 2025 +0100 @@ -48,8 +48,8 @@ ValueType type) = 0; }; + typedef std::map<std::string, ValueType> Parameters; private: - typedef std::map<std::string, ValueType> Parameters; class Token; @@ -57,14 +57,19 @@ Parameters parameters_; bool readOnly_; - void Setup(const std::string& sql); + void Setup(const std::string& sql, const Query::Parameters& parametersTypes); public: explicit Query(const std::string& sql); + explicit Query(const std::string& sql, const Parameters& parametersType); Query(const std::string& sql, bool isReadOnly); + Query(const std::string& sql, + bool isReadOnly, + const Parameters& parametersType); + ~Query(); bool IsReadOnly() const
--- a/Framework/Plugins/ISqlLookupFormatter.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Plugins/ISqlLookupFormatter.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -473,7 +473,7 @@ { std::string arg = "order" + boost::lexical_cast<std::string>(index); - target = " INNER JOIN Metadata " + arg + " ON " + arg + ".id = " + FormatLevel(requestLevel) + + target = " LEFT JOIN Metadata " + arg + " ON " + arg + ".id = " + FormatLevel(requestLevel) + ".internalId AND " + arg + ".type = " + boost::lexical_cast<std::string>(metadata); } @@ -1043,11 +1043,6 @@ comparisons += " AND " + comparison; } - if (!comparison.empty()) - { - comparisons += " AND " + comparison; - } - count ++; } }
--- a/Framework/Plugins/ISqlLookupFormatter.h Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Plugins/ISqlLookupFormatter.h Mon Mar 10 18:53:00 2025 +0100 @@ -56,6 +56,8 @@ virtual std::string GenerateParameter(const std::string& value) = 0; + virtual std::string GenerateParameter(const int64_t& value) = 0; + virtual std::string FormatResourceType(Orthanc::ResourceType level) = 0; virtual std::string FormatWildcardEscape() = 0;
--- a/Framework/Plugins/IndexBackend.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Plugins/IndexBackend.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -313,9 +313,11 @@ IndexBackend::IndexBackend(OrthancPluginContext* context, - bool readOnly) : + bool readOnly, + bool allowInconsistentChildCounts) : context_(context), - readOnly_(readOnly) + readOnly_(readOnly), + allowInconsistentChildCounts_(allowInconsistentChildCounts) { } @@ -2182,6 +2184,16 @@ return "${" + key + "}"; } + virtual std::string GenerateParameter(const int64_t& value) + { + const std::string key = FormatParameter(count_); + + count_ ++; + dictionary_.SetIntegerValue(key, value); + + return "${" + key + "}"; + } + virtual std::string FormatResourceType(Orthanc::ResourceType level) { return boost::lexical_cast<std::string>(MessagesToolbox::ConvertToPlainC(level)); @@ -2231,11 +2243,13 @@ { if (count > 0 || since > 0) { - sql += " OFFSET " + boost::lexical_cast<std::string>(since) + " ROWS "; + std::string parameterSince = GenerateParameter(since); + sql += " OFFSET " + parameterSince + " ROWS "; } if (count > 0) { - sql += " FETCH NEXT " + boost::lexical_cast<std::string>(count) + " ROWS ONLY "; + std::string parameterCount = GenerateParameter(count); + sql += " FETCH NEXT " + parameterCount + " ROWS ONLY "; } }; break; case Dialect_SQLite: @@ -2243,26 +2257,32 @@ { if (count > 0) { - sql += " LIMIT " + boost::lexical_cast<std::string>(count); + std::string parameterCount = GenerateParameter(count); + sql += " LIMIT " + parameterCount; } if (since > 0) { - sql += " OFFSET " + boost::lexical_cast<std::string>(since); + std::string parameterSince = GenerateParameter(since); + sql += " OFFSET " + parameterSince; } }; break; case Dialect_MySQL: { if (count > 0 && since > 0) { - sql += " LIMIT " + boost::lexical_cast<std::string>(since) + ", " + boost::lexical_cast<std::string>(count); + std::string parameterCount = GenerateParameter(count); + std::string parameterSince = GenerateParameter(since); + sql += " LIMIT " + parameterSince + ", " + parameterCount; } else if (count > 0) { - sql += " LIMIT " + boost::lexical_cast<std::string>(count); + std::string parameterCount = GenerateParameter(count); + sql += " LIMIT " + parameterCount; } else if (since > 0) { - sql += " LIMIT " + boost::lexical_cast<std::string>(since) + ", 18446744073709551615"; // max uint64 value when you don't want any limit + std::string parameterSince = GenerateParameter(since); + sql += " LIMIT " + parameterSince + ", 18446744073709551615"; // max uint64 value when you don't want any limit } }; break; default: @@ -2316,15 +2336,6 @@ } } - void PrepareStatement(DatabaseManager::StandaloneStatement& statement) const - { - statement.SetReadOnly(true); - - for (size_t i = 0; i < count_; i++) - { - statement.SetParameterType(FormatParameter(i), ValueType_Utf8String); - } - } const Dictionary& GetDictionary() const { @@ -2441,9 +2452,10 @@ } } - DatabaseManager::StandaloneStatement statement(manager, sql); - formatter.PrepareStatement(statement); - + Query::Parameters parametersTypes; + formatter.GetDictionary().GetParametersType(parametersTypes); + + DatabaseManager::StandaloneStatement statement(manager, sql, parametersTypes); statement.Execute(formatter.GetDictionary()); while (!statement.IsDone()) @@ -3289,7 +3301,10 @@ sql = "WITH Lookup AS (" + lookupSql + ") SELECT COUNT(*) FROM Lookup"; - DatabaseManager::CachedStatement statement(STATEMENT_FROM_HERE_DYNAMIC(sql), manager, sql); + Query::Parameters parametersTypes; + formatter.GetDictionary().GetParametersType(parametersTypes); + + DatabaseManager::CachedStatement statement(STATEMENT_FROM_HERE_DYNAMIC(sql), manager, sql, parametersTypes); statement.Execute(formatter.GetDictionary()); response.mutable_count_resources()->set_count(statement.ReadInteger64(0)); } @@ -3306,6 +3321,29 @@ // why we have generic column names. // So, at the end we'll have only one very big query ! + const Orthanc::DatabasePluginMessages::Find_Request_ChildrenSpecification* childrenSpec = NULL; + const Orthanc::DatabasePluginMessages::Find_Request_ChildrenSpecification* grandchildrenSpec = NULL; + const Orthanc::DatabasePluginMessages::Find_Request_ChildrenSpecification* grandgrandchildrenSpec = NULL; + + switch (request.level()) + { + case Orthanc::DatabasePluginMessages::RESOURCE_PATIENT: + childrenSpec = &(request.children_studies()); + grandchildrenSpec = &(request.children_series()); + grandgrandchildrenSpec = &(request.children_instances()); + break; + case Orthanc::DatabasePluginMessages::RESOURCE_STUDY: + childrenSpec = &(request.children_series()); + grandchildrenSpec = &(request.children_instances()); + break; + case Orthanc::DatabasePluginMessages::RESOURCE_SERIES: + childrenSpec = &(request.children_instances()); + break; + + default: + break; + } + std::string sql; // extract the resource id of interest by executing the lookup in a CTE @@ -3318,6 +3356,7 @@ std::string oneInstanceSqlCTE; + // OneInstance CTE if (request.level() != Orthanc::DatabasePluginMessages::RESOURCE_INSTANCE && request.retrieve_one_instance_metadata_and_attachments()) { @@ -3351,10 +3390,47 @@ sql += ", OneInstance AS (SELECT parentInternalId, instancePublicId, instanceInternalId FROM _OneInstance WHERE rowNum = 1) "; // this is a generic way to implement DISTINCT ON } - // if (!oneInstanceSqlCTE.empty() && (manager.GetDialect() == Dialect_MySQL || manager.GetDialect() == Dialect_SQLite)) - // { // all CTEs must be declared first in some dialects - // } - + // GrandChildCount CTE in some specific cases + bool hasGrandChildCountCTE = false; + if (HasChildCountColumn() && !allowInconsistentChildCounts_ && + grandchildrenSpec != NULL && + !grandchildrenSpec->retrieve_identifiers() && // no need to count if we have retrieved the list of identifiers + grandchildrenSpec->retrieve_count()) + { + sql += ", ValidGrandChildCounts AS (SELECT thisLevel.internalId, COALESCE(SUM(childLevel.childCount), 0) AS totalGrandChildCount " + " FROM Resources AS thisLevel " + " LEFT JOIN Resources AS childLevel ON childLevel.parentId = thisLevel.internalId " + " WHERE NOT EXISTS (SELECT 1 FROM InvalidChildCounts WHERE id = thisLevel.internalId) " + " AND NOT EXISTS (SELECT 1 FROM Resources AS child " + " JOIN InvalidChildCounts ON InvalidChildCounts.id = child.internalId " + " WHERE child.parentId = thisLevel.internalId) " + " GROUP BY thisLevel.internalId) "; + hasGrandChildCountCTE = true; + } + + // GrandGrandChildCount CTE in some specific cases + bool hasGrandGrandChildCountCTE = false; + if (HasChildCountColumn() && !allowInconsistentChildCounts_ && + grandgrandchildrenSpec != NULL && + !grandgrandchildrenSpec->retrieve_identifiers() && // no need to count if we have retrieved the list of identifiers + grandgrandchildrenSpec->retrieve_count()) + { + sql += ", ValidGrandGrandChildCounts AS (SELECT thisLevel.internalId, COALESCE(SUM(grandChildLevel.childCount), 0) AS totalGrandGrandChildCount " + " FROM Resources AS thisLevel " + " LEFT JOIN Resources AS childLevel ON childLevel.parentId = thisLevel.internalId " + " LEFT JOIN Resources AS grandChildLevel ON grandChildLevel.parentId = childLevel.internalId " + " WHERE NOT EXISTS (SELECT 1 FROM InvalidChildCounts WHERE id = thisLevel.internalId) " + " AND NOT EXISTS (SELECT 1 FROM Resources AS child " + " JOIN InvalidChildCounts ON InvalidChildCounts.id = child.internalId " + " WHERE child.parentId = thisLevel.internalId) " + " AND NOT EXISTS (SELECT 1 FROM Resources AS grandChild " + " JOIN Resources AS child ON grandChild.parentId = child.internalId " + " JOIN InvalidChildCounts ON InvalidChildCounts.id = grandChild.internalId " + " WHERE child.parentId = thisLevel.internalId) " + " GROUP BY thisLevel.internalId) "; + hasGrandGrandChildCountCTE = true; + } + std::string revisionInC7; if (HasRevisionsSupport()) { @@ -3585,22 +3661,7 @@ // need MainDicomTags from children ? if (request.level() <= Orthanc::DatabasePluginMessages::RESOURCE_SERIES) { - const Orthanc::DatabasePluginMessages::Find_Request_ChildrenSpecification* childrenSpec = NULL; - switch (request.level()) - { - case Orthanc::DatabasePluginMessages::RESOURCE_PATIENT: - childrenSpec = &(request.children_studies()); - break; - case Orthanc::DatabasePluginMessages::RESOURCE_STUDY: - childrenSpec = &(request.children_series()); - break; - case Orthanc::DatabasePluginMessages::RESOURCE_SERIES: - childrenSpec = &(request.children_instances()); - break; - - default: - break; - } + assert(childrenSpec != NULL); if (childrenSpec->retrieve_main_dicom_tags_size() > 0) { @@ -3643,30 +3704,24 @@ } else if (childrenSpec->retrieve_count()) // no need to count if we have retrieved the list of identifiers { - if (HasChildCountTable()) // TODO: rename in HasChildCountColumn ? + if (HasChildCountColumn()) { - // // we get the count value either from the childCount table if it has been computed or from the Resources table - // sql += "UNION ALL SELECT " - // " " TOSTRING(QUERY_CHILDREN_COUNT) " AS c0_queryId, " - // " Lookup.internalId AS c1_internalId, " - // " " + formatter.FormatNull("BIGINT") + " AS c2_rowNumber, " - // " " + formatter.FormatNull("TEXT") + " AS c3_string1, " - // " " + formatter.FormatNull("TEXT") + " AS c4_string2, " - // " " + formatter.FormatNull("TEXT") + " AS c5_string3, " - // " " + formatter.FormatNull("INT") + " AS c7_int1, " - // " " + formatter.FormatNull("INT") + " AS c8_int2, " - // " " + formatter.FormatNull("INT") + " AS c9_int3, " - // " COALESCE(" - // " (ChildCount.childCount)," - // " (SELECT COUNT(childLevel.internalId)" - // " FROM Resources AS childLevel" - // " WHERE Lookup.internalId = childLevel.parentId" - // " )) AS c10_big_int1, " - // " " + formatter.FormatNull("BIGINT") + " AS c11_big_int2 " - // "FROM Lookup " - // "LEFT JOIN ChildCount ON Lookup.internalId = ChildCount.parentId "; - // we get the count value either from the childCount column if it has been computed or from the Resources table + std::string getResourcesChildCount; + + if (allowInconsistentChildCounts_) // allow approximate childCount if childCount is populated but has not yet been updated + { + getResourcesChildCount = "Resources.childCount"; // read the last known value without taking into account the InvalidChildCounts table to speed up the query + } + else + { + // read the last known value only if it has not been invalidated by an entry in the InvalidChildCounts table -> more complex/slower query + getResourcesChildCount = "SELECT childCount " + "FROM Resources AS thisLevel " + "WHERE thisLevel.internalId = lookup.internalId " + " AND NOT EXISTS (SELECT 1 FROM Invalidchildcounts WHERE id = thisLevel.internalId)"; + } + sql += "UNION ALL SELECT " " " TOSTRING(QUERY_CHILDREN_COUNT) " AS c0_queryId, " " Lookup.internalId AS c1_internalId, " @@ -3679,7 +3734,7 @@ " " + formatter.FormatNull("INT") + " AS c8_int2, " " " + formatter.FormatNull("INT") + " AS c9_int3, " " COALESCE(" - " (Resources.childCount)," + " (" + getResourcesChildCount + ")," " (SELECT COUNT(childLevel.internalId)" " FROM Resources AS childLevel" " WHERE Lookup.internalId = childLevel.parentId" @@ -3730,19 +3785,7 @@ if (request.level() <= Orthanc::DatabasePluginMessages::RESOURCE_STUDY) { - const Orthanc::DatabasePluginMessages::Find_Request_ChildrenSpecification* grandchildrenSpec = NULL; - switch (request.level()) - { - case Orthanc::DatabasePluginMessages::RESOURCE_PATIENT: - grandchildrenSpec = &(request.children_series()); - break; - case Orthanc::DatabasePluginMessages::RESOURCE_STUDY: - grandchildrenSpec = &(request.children_instances()); - break; - - default: - break; - } + assert(grandchildrenSpec != NULL); // need grand children identifiers ? if (grandchildrenSpec->retrieve_identifiers()) @@ -3766,55 +3809,51 @@ } else if (grandchildrenSpec->retrieve_count()) // no need to count if we have retrieved the list of identifiers { - if (HasChildCountTable()) + if (HasChildCountColumn()) { - // // we get the count value either from the childCount table if it has been computed or from the Resources table - // sql += "UNION ALL SELECT " - // " " TOSTRING(QUERY_GRAND_CHILDREN_COUNT) " AS c0_queryId, " - // " Lookup.internalId AS c1_internalId, " - // " " + formatter.FormatNull("BIGINT") + " AS c2_rowNumber, " - // " " + formatter.FormatNull("TEXT") + " AS c3_string1, " - // " " + formatter.FormatNull("TEXT") + " AS c4_string2, " - // " " + formatter.FormatNull("TEXT") + " AS c5_string3, " - // " " + formatter.FormatNull("INT") + " AS c7_int1, " - // " " + formatter.FormatNull("INT") + " AS c8_int2, " - // " " + formatter.FormatNull("INT") + " AS c9_int3, " - // " COALESCE(" - // " (SELECT SUM(ChildCount.childCount)" - // " FROM ChildCount" - // " INNER JOIN Resources AS childLevel ON childLevel.parentId = Lookup.internalId" - // " WHERE ChildCount.parentId = childLevel.internalId)," - // " (SELECT COUNT(grandChildLevel.internalId)" - // " FROM Resources AS childLevel" - // " INNER JOIN Resources AS grandChildLevel ON childLevel.internalId = grandChildLevel.parentId" - // " WHERE Lookup.internalId = childLevel.parentId" - // " )) AS c10_big_int1, " - // " " + formatter.FormatNull("BIGINT") + " AS c11_big_int2 " - // "FROM Lookup "; + + // we get the count value either from the childCount column if it has been computed or from the Resources table + std::string getResourcesGrandChildCount; + + if (allowInconsistentChildCounts_) // allow approximate childCount if childCount is populated but has not yet been updated + { + // read the last known value without taking into account the InvalidChildCounts table to speed up the query + getResourcesGrandChildCount = "SELECT SUM(childLevel.childCount) " + "FROM Resources AS childLevel " + "WHERE childLevel.parentId = Lookup.internalId"; + } + else + { + assert(hasGrandChildCountCTE); + // read the last known value only if it has not been invalidated by an entry in the InvalidChildCounts table -> more complex/slower query + getResourcesGrandChildCount = "SELECT totalGrandChildCount " + "FROM ValidGrandChildCounts " + "WHERE internalId = lookup.internalId"; + //" AND NOT EXISTS (SELECT 1 FROM Invalidchildcounts WHERE id = childLevel.internalId OR id = lookup.internalId)"; + } // we get the count value either from the childCount column if it has been computed or from the Resources table sql += "UNION ALL SELECT " - " " TOSTRING(QUERY_GRAND_CHILDREN_COUNT) " AS c0_queryId, " - " Lookup.internalId AS c1_internalId, " - " " + formatter.FormatNull("BIGINT") + " AS c2_rowNumber, " - " " + formatter.FormatNull("TEXT") + " AS c3_string1, " - " " + formatter.FormatNull("TEXT") + " AS c4_string2, " - " " + formatter.FormatNull("TEXT") + " AS c5_string3, " - " " + formatter.FormatNull("TEXT") + " AS c6_string4, " - " " + formatter.FormatNull("INT") + " AS c7_int1, " - " " + formatter.FormatNull("INT") + " AS c8_int2, " - " " + formatter.FormatNull("INT") + " AS c9_int3, " - " COALESCE(" - " (SELECT SUM(childLevel.childCount)" - " FROM Resources AS childLevel" - " WHERE childLevel.parentId = Lookup.internalId)," - " (SELECT COUNT(grandChildLevel.internalId)" - " FROM Resources AS childLevel" - " INNER JOIN Resources AS grandChildLevel ON childLevel.internalId = grandChildLevel.parentId" - " WHERE Lookup.internalId = childLevel.parentId" - " )) AS c10_big_int1, " - " " + formatter.FormatNull("BIGINT") + " AS c11_big_int2 " - "FROM Lookup "; + + " " TOSTRING(QUERY_GRAND_CHILDREN_COUNT) " AS c0_queryId, " + " Lookup.internalId AS c1_internalId, " + " " + formatter.FormatNull("BIGINT") + " AS c2_rowNumber, " + " " + formatter.FormatNull("TEXT") + " AS c3_string1, " + " " + formatter.FormatNull("TEXT") + " AS c4_string2, " + " " + formatter.FormatNull("TEXT") + " AS c5_string3, " + " " + formatter.FormatNull("TEXT") + " AS c6_string4, " + " " + formatter.FormatNull("INT") + " AS c7_int1, " + " " + formatter.FormatNull("INT") + " AS c8_int2, " + " " + formatter.FormatNull("INT") + " AS c9_int3, " + " COALESCE(" + " (" + getResourcesGrandChildCount + ")," + " (SELECT COUNT(grandChildLevel.internalId)" + " FROM Resources AS childLevel" + " INNER JOIN Resources AS grandChildLevel ON childLevel.internalId = grandChildLevel.parentId" + " WHERE Lookup.internalId = childLevel.parentId" + " )) AS c10_big_int1, " + " " + formatter.FormatNull("BIGINT") + " AS c11_big_int2 " + "FROM Lookup "; } else { @@ -3881,7 +3920,7 @@ if (request.level() == Orthanc::DatabasePluginMessages::RESOURCE_PATIENT) { - const Orthanc::DatabasePluginMessages::Find_Request_ChildrenSpecification* grandgrandchildrenSpec = &(request.children_instances()); + assert(grandgrandchildrenSpec != NULL); // need grand children identifiers ? if (grandgrandchildrenSpec->retrieve_identifiers()) @@ -3906,33 +3945,29 @@ } else if (grandgrandchildrenSpec->retrieve_count()) // no need to count if we have retrieved the list of identifiers { - if (HasChildCountTable()) + if (HasChildCountColumn()) { - // // we get the count value either from the childCount table if it has been computed or from the Resources table - // sql += "UNION ALL SELECT " - // " " TOSTRING(QUERY_GRAND_GRAND_CHILDREN_COUNT) " AS c0_queryId, " - // " Lookup.internalId AS c1_internalId, " - // " " + formatter.FormatNull("BIGINT") + " AS c2_rowNumber, " - // " " + formatter.FormatNull("TEXT") + " AS c3_string1, " - // " " + formatter.FormatNull("TEXT") + " AS c4_string2, " - // " " + formatter.FormatNull("TEXT") + " AS c5_string3, " - // " " + formatter.FormatNull("INT") + " AS c7_int1, " - // " " + formatter.FormatNull("INT") + " AS c8_int2, " - // " " + formatter.FormatNull("INT") + " AS c9_int3, " - // " COALESCE(" - // " (SELECT SUM(ChildCount.childCount)" - // " FROM ChildCount" - // " INNER JOIN Resources AS childLevel ON childLevel.parentId = Lookup.internalId" - // " INNER JOIN Resources AS grandChildLevel ON grandChildLevel.parentId = childLevel.internalId" - // " WHERE ChildCount.parentId = grandChildLevel.internalId)," - // " (SELECT COUNT(grandGrandChildLevel.internalId)" - // " FROM Resources AS childLevel" - // " INNER JOIN Resources AS grandChildLevel ON childLevel.internalId = grandChildLevel.parentId" - // " INNER JOIN Resources AS grandGrandChildLevel ON grandChildLevel.internalId = grandGrandChildLevel.parentId" - // " WHERE Lookup.internalId = childLevel.parentId" - // " )) AS c10_big_int1, " - // " " + formatter.FormatNull("BIGINT") + " AS c11_big_int2 " - // "FROM Lookup "; + std::string getResourcesGrandGrandChildCount; + if (allowInconsistentChildCounts_) // allow approximate childCount if childCount is populated but has not yet been updated + { + // read the last known value without taking into account the InvalidChildCounts table to speed up the query + getResourcesGrandGrandChildCount = "SELECT SUM(grandChildLevel.childCount)" + "FROM Resources AS grandChildLevel" + "INNER JOIN Resources AS childLevel ON childLevel.parentId = Lookup.internalId" + "WHERE grandChildLevel.parentId = childLevel.internalId"; + } + else + { + assert(hasGrandGrandChildCountCTE); + // read the last known value only if it has not been invalidated by an entry in the InvalidChildCounts table -> more complex/slower query + getResourcesGrandGrandChildCount = "SELECT totalGrandGrandChildCount " + "FROM ValidGrandGrandChildCounts " + "WHERE internalId = lookup.internalId"; + // "INNER JOIN Resources AS childLevel ON childLevel.parentId = Lookup.internalId" + // "WHERE grandChildLevel.parentId = childLevel.internalId" + // " AND NOT EXISTS (SELECT 1 FROM Invalidchildcounts WHERE id = grandChildLevel.internalId OR id = childLevel.internalId OR id = lookup.internalId)"; + } + // we get the count value either from the childCount column if it has been computed or from the Resources table sql += "UNION ALL SELECT " @@ -3947,10 +3982,7 @@ " " + formatter.FormatNull("INT") + " AS c8_int2, " " " + formatter.FormatNull("INT") + " AS c9_int3, " " COALESCE(" - " (SELECT SUM(grandChildLevel.childCount)" - " FROM Resources AS grandChildLevel" - " INNER JOIN Resources AS childLevel ON childLevel.parentId = Lookup.internalId" - " WHERE grandChildLevel.parentId = childLevel.internalId)," + " (" + getResourcesGrandGrandChildCount + ")," " (SELECT COUNT(grandGrandChildLevel.internalId)" " FROM Resources AS childLevel" " INNER JOIN Resources AS grandChildLevel ON childLevel.internalId = grandChildLevel.parentId" @@ -4064,13 +4096,16 @@ sql += " ORDER BY c0_queryId, c2_rowNumber"; // this is really important to make sure that the Lookup query is the first one to provide results since we use it to create the responses element ! std::unique_ptr<DatabaseManager::StatementBase> statement; + + Query::Parameters parametersTypes; + formatter.GetDictionary().GetParametersType(parametersTypes); if (manager.GetDialect() == Dialect_MySQL) { // TODO: investigate why "complex" cached statement do not seem to work properly in MySQL - statement.reset(new DatabaseManager::StandaloneStatement(manager, sql)); + statement.reset(new DatabaseManager::StandaloneStatement(manager, sql, parametersTypes)); } else { - statement.reset(new DatabaseManager::CachedStatement(STATEMENT_FROM_HERE_DYNAMIC(sql), manager, sql)); + statement.reset(new DatabaseManager::CachedStatement(STATEMENT_FROM_HERE_DYNAMIC(sql), manager, sql, parametersTypes)); } statement->Execute(formatter.GetDictionary());
--- a/Framework/Plugins/IndexBackend.h Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Plugins/IndexBackend.h Mon Mar 10 18:53:00 2025 +0100 @@ -43,18 +43,20 @@ OrthancPluginContext* context_; bool readOnly_; + bool allowInconsistentChildCounts_; boost::shared_mutex outputFactoryMutex_; std::unique_ptr<IDatabaseBackendOutput::IFactory> outputFactory_; protected: + virtual void ClearDeletedFiles(DatabaseManager& manager); virtual void ClearDeletedResources(DatabaseManager& manager); virtual void ClearRemainingAncestor(DatabaseManager& manager); - virtual bool HasChildCountTable() const = 0; + virtual bool HasChildCountColumn() const = 0; void SignalDeletedFiles(IDatabaseBackendOutput& output, DatabaseManager& manager); @@ -84,7 +86,8 @@ public: explicit IndexBackend(OrthancPluginContext* context, - bool readOnly); + bool readOnly, + bool allowInconsistentChildCounts); virtual OrthancPluginContext* GetContext() ORTHANC_OVERRIDE {
--- a/Framework/Plugins/IndexConnectionsPool.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/Plugins/IndexConnectionsPool.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -49,6 +49,10 @@ void IndexConnectionsPool::HousekeepingThread(IndexConnectionsPool* that) { +#if ORTHANC_PLUGINS_VERSION_IS_ABOVE(1, 12, 2) + OrthancPluginSetCurrentThreadName(OrthancPlugins::GetGlobalContext(), "DB HOUSEKEEPING"); +#endif + boost::posix_time::ptime lastInvocation = boost::posix_time::second_clock::local_time(); while (that->housekeepingContinue_)
--- a/Framework/PostgreSQL/PostgreSQLParameters.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/PostgreSQL/PostgreSQLParameters.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -98,6 +98,7 @@ lock_ = configuration.GetBooleanValue("Lock", true); // Use locking by default isVerboseEnabled_ = configuration.GetBooleanValue("EnableVerboseLogs", false); + allowInconsistentChildCounts_ = configuration.GetBooleanValue("AllowInconsistentChildCounts", false); maxConnectionRetries_ = configuration.GetUnsignedIntegerValue("MaximumConnectionRetries", 10); connectionRetryInterval_ = configuration.GetUnsignedIntegerValue("ConnectionRetryInterval", 5);
--- a/Framework/PostgreSQL/PostgreSQLParameters.h Tue Feb 04 17:12:58 2025 +0100 +++ b/Framework/PostgreSQL/PostgreSQLParameters.h Mon Mar 10 18:53:00 2025 +0100 @@ -51,6 +51,7 @@ unsigned int maxConnectionRetries_; unsigned int connectionRetryInterval_; bool isVerboseEnabled_; + bool allowInconsistentChildCounts_; IsolationMode isolationMode_; void Reset(); @@ -152,6 +153,10 @@ return isVerboseEnabled_; } + bool GetAllowInconsistentChildCounts() const + { + return allowInconsistentChildCounts_; + } void Format(std::string& target) const; };
--- a/MySQL/Plugins/MySQLIndex.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/MySQL/Plugins/MySQLIndex.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -41,7 +41,7 @@ MySQLIndex::MySQLIndex(OrthancPluginContext* context, const MySQLParameters& parameters, bool readOnly) : - IndexBackend(context, readOnly), + IndexBackend(context, readOnly, false), parameters_(parameters), clearAll_(false) {
--- a/MySQL/Plugins/MySQLIndex.h Tue Feb 04 17:12:58 2025 +0100 +++ b/MySQL/Plugins/MySQLIndex.h Mon Mar 10 18:53:00 2025 +0100 @@ -35,7 +35,7 @@ bool clearAll_; protected: - virtual bool HasChildCountTable() const ORTHANC_OVERRIDE + virtual bool HasChildCountColumn() const ORTHANC_OVERRIDE { return false; }
--- a/Odbc/Plugins/OdbcIndex.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/Odbc/Plugins/OdbcIndex.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -137,7 +137,7 @@ OdbcIndex::OdbcIndex(OrthancPluginContext* context, const std::string& connectionString, bool readOnly) : - IndexBackend(context, readOnly), + IndexBackend(context, readOnly, false), maxConnectionRetries_(10), connectionRetryInterval_(5), connectionString_(connectionString)
--- a/Odbc/Plugins/OdbcIndex.h Tue Feb 04 17:12:58 2025 +0100 +++ b/Odbc/Plugins/OdbcIndex.h Mon Mar 10 18:53:00 2025 +0100 @@ -35,7 +35,7 @@ std::string connectionString_; protected: - virtual bool HasChildCountTable() const ORTHANC_OVERRIDE + virtual bool HasChildCountColumn() const ORTHANC_OVERRIDE { return false; }
--- a/PostgreSQL/CMakeLists.txt Tue Feb 04 17:12:58 2025 +0100 +++ b/PostgreSQL/CMakeLists.txt Mon Mar 10 18:53:00 2025 +0100 @@ -94,6 +94,7 @@ POSTGRESQL_UPGRADE_REV1_TO_REV2 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev1ToRev2.sql POSTGRESQL_UPGRADE_REV2_TO_REV3 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev2ToRev3.sql POSTGRESQL_UPGRADE_REV3_TO_REV4 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev3ToRev4.sql + POSTGRESQL_UPGRADE_REV4_TO_REV5 ${CMAKE_SOURCE_DIR}/Plugins/SQL/Upgrades/Rev4ToRev5.sql )
--- a/PostgreSQL/NEWS Tue Feb 04 17:12:58 2025 +0100 +++ b/PostgreSQL/NEWS Mon Mar 10 18:53:00 2025 +0100 @@ -17,6 +17,34 @@ +Release 7.2 (2025-02-27) +======================== + +DB schema revision: 4 +Minimum plugin SDK (for build): 1.12.5 +Optimal plugin SDK (for build): 1.12.5 +Minimum Orthanc runtime: 1.12.5 +Optimal Orthanc runtime: 1.12.6 + +Minimal Postgresql Server version: 9 +Optimal Postgresql Server version: 11+ + + +Maintenance: +* Optimized the SQL query that is maintaing the childCount + when new instances are ingested. +* New configuration "AllowInconsistentChildCounts" to speed + up childCount computation. If set to true, childCount values + of recently ingested resources will be incorrect until the next + execution of the DB Housekeeping thread (defined by "HousekeepingInterval" + - default value is 1s) +* Fixed high memory usage due to caching of too many + prepared SQL statement when using since & limit. +* Removed duplicate comparison in find SQL queries. +* Now returning results when e.g, ordering instances against a metadata they don't have. + + + Release 7.1 (2025-01-23) ========================
--- a/PostgreSQL/Plugins/PostgreSQLIndex.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/PostgreSQL/Plugins/PostgreSQLIndex.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -49,13 +49,14 @@ static const GlobalProperty GlobalProperty_HasComputeStatisticsReadOnly = GlobalProperty_DatabaseInternal4; } +#define CURRENT_DB_REVISION 5 namespace OrthancDatabases { PostgreSQLIndex::PostgreSQLIndex(OrthancPluginContext* context, const PostgreSQLParameters& parameters, bool readOnly) : - IndexBackend(context, readOnly), + IndexBackend(context, readOnly, parameters.GetAllowInconsistentChildCounts()), parameters_(parameters), clearAll_(false), hkHasComputedAllMissingChildCount_(false) @@ -122,6 +123,7 @@ { DatabaseManager::Transaction t(manager, TransactionType_ReadWrite); + bool hasAppliedAnUpgrade = false; if (!t.GetDatabaseTransaction().DoesTableExist("Resources")) { @@ -147,40 +149,12 @@ throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); } - bool applyUpgradeFromUnknownToV1 = false; - bool applyUpgradeV1toV2 = false; - bool applyUpgradeV2toV3 = false; - bool applyUpgradeV3toV4 = false; - bool applyPrepareIndex = false; - - int revision; - if (!LookupGlobalIntegerProperty(revision, manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_DatabasePatchLevel)) + int currentRevision = 0; + if (!LookupGlobalIntegerProperty(currentRevision, manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_DatabasePatchLevel)) { - LOG(WARNING) << "No DatabasePatchLevel found, assuming it's 1"; - revision = 1; - applyUpgradeFromUnknownToV1 = true; - applyUpgradeV1toV2 = true; - applyUpgradeV2toV3 = true; - applyUpgradeV3toV4 = true; + LOG(WARNING) << "No Database revision found"; } - else if (revision == 1) - { - LOG(WARNING) << "DatabasePatchLevel is 1"; - applyUpgradeV1toV2 = true; - applyUpgradeV2toV3 = true; - applyUpgradeV3toV4 = true; - } - else if (revision == 2) - { - LOG(WARNING) << "DatabasePatchLevel is 2"; - applyUpgradeV2toV3 = true; - applyUpgradeV3toV4 = true; - } - else if (revision == 3) - { - LOG(WARNING) << "DatabasePatchLevel is 3"; - applyUpgradeV3toV4 = true; - } + LOG(WARNING) << "Current Database revision is " << currentRevision; int hasTrigram = 0; if (!LookupGlobalIntegerProperty(hasTrigram, manager, MISSING_SERVER_IDENTIFIER, @@ -189,10 +163,12 @@ { // We've observed 9 minutes on DB with 100000 studies LOG(WARNING) << "The DB schema update will try to enable trigram matching on the PostgreSQL database " - << "to speed up wildcard searches. This may take several minutes"; - applyUpgradeV1toV2 = true; - applyUpgradeV2toV3 = true; - applyUpgradeV3toV4 = true; + << "to speed up wildcard searches. This may take several minutes. "; + if (currentRevision > 0) + { + LOG(WARNING) << "Considering current revision is 1"; + currentRevision = 1; + } } int property = 0; @@ -200,16 +176,18 @@ Orthanc::GlobalProperty_GetLastChangeIndex) || property != 1) { - applyUpgradeV1toV2 = true; - applyUpgradeV2toV3 = true; - applyUpgradeV3toV4 = true; + LOG(WARNING) << "The DB schema does not contain the GetLastChangeIndex update. "; + if (currentRevision > 0) + { + LOG(WARNING) << "Considering current revision is 1"; + currentRevision = 1; + } + } // If you add new tests here, update the test in the "ReadOnly" code below - applyPrepareIndex = applyUpgradeV3toV4; - - if (applyUpgradeFromUnknownToV1) + if (currentRevision == 0) { LOG(WARNING) << "Upgrading DB schema from unknown to revision 1"; std::string query; @@ -217,9 +195,10 @@ Orthanc::EmbeddedResources::GetFileResource (query, Orthanc::EmbeddedResources::POSTGRESQL_UPGRADE_UNKNOWN_TO_REV1); t.GetDatabaseTransaction().ExecuteMultiLines(query); + currentRevision = 1; } - if (applyUpgradeV1toV2) + if (currentRevision == 1) { LOG(WARNING) << "Upgrading DB schema from revision 1 to revision 2"; @@ -228,9 +207,10 @@ Orthanc::EmbeddedResources::GetFileResource (query, Orthanc::EmbeddedResources::POSTGRESQL_UPGRADE_REV1_TO_REV2); t.GetDatabaseTransaction().ExecuteMultiLines(query); + currentRevision = 2; } - if (applyUpgradeV2toV3) + if (currentRevision == 2) { LOG(WARNING) << "Upgrading DB schema from revision 2 to revision 3"; @@ -239,9 +219,10 @@ Orthanc::EmbeddedResources::GetFileResource (query, Orthanc::EmbeddedResources::POSTGRESQL_UPGRADE_REV2_TO_REV3); t.GetDatabaseTransaction().ExecuteMultiLines(query); + currentRevision = 3; } - if (applyUpgradeV3toV4) + if (currentRevision == 3) { LOG(WARNING) << "Upgrading DB schema from revision 3 to revision 4"; @@ -250,17 +231,62 @@ Orthanc::EmbeddedResources::GetFileResource (query, Orthanc::EmbeddedResources::POSTGRESQL_UPGRADE_REV3_TO_REV4); t.GetDatabaseTransaction().ExecuteMultiLines(query); + hasAppliedAnUpgrade = true; + currentRevision = 4; + } + + if (currentRevision == 4) + { + LOG(WARNING) << "Upgrading DB schema from revision 4 to revision 5"; + + std::string query; + + Orthanc::EmbeddedResources::GetFileResource + (query, Orthanc::EmbeddedResources::POSTGRESQL_UPGRADE_REV4_TO_REV5); + t.GetDatabaseTransaction().ExecuteMultiLines(query); + hasAppliedAnUpgrade = true; + currentRevision = 5; } - if (applyPrepareIndex) + if (hasAppliedAnUpgrade) { + LOG(WARNING) << "Upgrading DB schema by applying PrepareIndex.sql"; // apply all idempotent changes that are in the PrepareIndex.sql ApplyPrepareIndex(t, manager); + + if (!LookupGlobalIntegerProperty(currentRevision, manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_DatabasePatchLevel)) + { + LOG(ERROR) << "No Database revision found after the upgrade !"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); + } + + LOG(WARNING) << "Database revision after the upgrade is " << currentRevision; + + if (currentRevision != CURRENT_DB_REVISION) + { + LOG(ERROR) << "Invalid database revision after the upgrade !"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); + } } } t.Commit(); + + if (hasAppliedAnUpgrade) + { + int currentRevision = 0; + + LookupGlobalIntegerProperty(currentRevision, manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_DatabasePatchLevel); + LOG(WARNING) << "Database revision after the upgrade is " << currentRevision; + + if (currentRevision != CURRENT_DB_REVISION) + { + LOG(ERROR) << "Invalid database revision after the upgrade !"; + throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); + } + } + } } else @@ -272,7 +298,7 @@ // test if the latest "extension" has been installed int revision; if (!LookupGlobalIntegerProperty(revision, manager, MISSING_SERVER_IDENTIFIER, Orthanc::GlobalProperty_DatabasePatchLevel) - || revision != 3) + || revision != CURRENT_DB_REVISION) { LOG(ERROR) << "READ-ONLY SYSTEM: the DB does not have the correct schema to run with this version of the plugin"; throw Orthanc::OrthancException(Orthanc::ErrorCode_Database); @@ -790,5 +816,26 @@ int64_t patientsCount, studiesCount, seriesCount, instancesCount, compressedSize, uncompressedSize; UpdateAndGetStatistics(manager, patientsCount, studiesCount, seriesCount, instancesCount, compressedSize, uncompressedSize); } + + // Update the invalidated childCounts + try + { + DatabaseManager::CachedStatement statement( + STATEMENT_FROM_HERE, manager, + "SELECT * FROM UpdateInvalidChildCounts()"); + + statement.Execute(); + + int64_t updatedCount = statement.ReadInteger64(0); + if (updatedCount > 0) + { + LOG(INFO) << "Updated " << updatedCount << " invalid ChildCount entries"; + } + } + catch (Orthanc::OrthancException&) + { + // the statement may fail in case of temporary deadlock -> it will be retried at the next HK + LOG(INFO) << "Updat of invalid ChildCount entries has failed (will be retried)"; + } } }
--- a/PostgreSQL/Plugins/PostgreSQLIndex.h Tue Feb 04 17:12:58 2025 +0100 +++ b/PostgreSQL/Plugins/PostgreSQLIndex.h Mon Mar 10 18:53:00 2025 +0100 @@ -44,7 +44,7 @@ virtual void ClearRemainingAncestor(DatabaseManager& manager) ORTHANC_OVERRIDE; - virtual bool HasChildCountTable() const ORTHANC_OVERRIDE + virtual bool HasChildCountColumn() const ORTHANC_OVERRIDE { return true; }
--- a/PostgreSQL/Plugins/SQL/Downgrades/Rev4ToRev3.sql Tue Feb 04 17:12:58 2025 +0100 +++ b/PostgreSQL/Plugins/SQL/Downgrades/Rev4ToRev3.sql Mon Mar 10 18:53:00 2025 +0100 @@ -1,56 +1,87 @@ -- This file contains an SQL procedure to downgrade from schema Rev4 to Rev3 (version = 6). --- It removes the column that has been added in Rev4 - --- these constraints were introduced in Rev4 -ALTER TABLE AttachedFiles DROP COLUMN customData; +-- It re-installs the old childcount trigger mechanisms --- reinstall previous triggers -CREATE OR REPLACE FUNCTION CreateDeletedFilesTemporaryTable( -) RETURNS VOID AS $body$ +DO $$ +DECLARE + current_revision TEXT; + expected_revision TEXT; +BEGIN + expected_revision := '4'; -BEGIN + SELECT value INTO current_revision FROM GlobalProperties WHERE property = 4; -- GlobalProperty_DatabasePatchLevel - SET client_min_messages = warning; -- suppress NOTICE: relation "deletedresources" already exists, skipping - - -- note: temporary tables are created at session (connection) level -> they are likely to exist - CREATE TEMPORARY TABLE IF NOT EXISTS DeletedFiles( - uuid VARCHAR(64) NOT NULL, - fileType INTEGER, - compressedSize BIGINT, - uncompressedSize BIGINT, - compressionType INTEGER, - uncompressedHash VARCHAR(40), - compressedHash VARCHAR(40) - ); + IF current_revision != expected_revision THEN + RAISE EXCEPTION 'Unexpected schema revision % to run this script. Expected revision = %', current_revision, expected_revision; + END IF; +END $$; - RESET client_min_messages; +--- - -- clear the temporary table in case it has been created earlier in the session - DELETE FROM DeletedFiles; -END; +DROP TRIGGER IF EXISTS IncrementChildCount on Resources; +DROP TRIGGER IF EXISTS DecrementChildCount on Resources; -$body$ LANGUAGE plpgsql; - - -CREATE OR REPLACE FUNCTION AttachedFileDeletedFunc() +CREATE OR REPLACE FUNCTION UpdateChildCount() RETURNS TRIGGER AS $body$ BEGIN - INSERT INTO DeletedFiles VALUES - (old.uuid, old.filetype, old.compressedSize, - old.uncompressedSize, old.compressionType, - old.uncompressedHash, old.compressedHash); - RETURN NULL; + IF TG_OP = 'INSERT' THEN + IF new.parentId IS NOT NULL THEN + -- try to increment the childcount from the parent + -- note that we already have the lock on this row because the parent is locked in CreateInstance + UPDATE Resources + SET childCount = childCount + 1 + WHERE internalId = new.parentId AND childCount IS NOT NULL; + + -- this should only happen for old studies whose childCount has not yet been computed + -- note: this child has already been added so it will be counted + IF NOT FOUND THEN + UPDATE Resources + SET childCount = (SELECT COUNT(*) + FROM Resources + WHERE internalId = new.parentId) + WHERE internalId = new.parentId; + END IF; + END IF; + + ELSIF TG_OP = 'DELETE' THEN + + IF old.parentId IS NOT NULL THEN + + -- Decrement the child count for the parent + -- note that we already have the lock on this row because the parent is locked in DeleteResource + UPDATE Resources + SET childCount = childCount - 1 + WHERE internalId = old.parentId AND childCount IS NOT NULL; + + -- this should only happen for old studies whose childCount has not yet been computed + -- note: this child has already been removed so it will not be counted + IF NOT FOUND THEN + UPDATE Resources + SET childCount = (SELECT COUNT(*) + FROM Resources + WHERE internalId = new.parentId) + WHERE internalId = new.parentId; + END IF; + END IF; + + END IF; + RETURN NULL; END; $body$ LANGUAGE plpgsql; -DROP TRIGGER IF EXISTS AttachedFileDeleted on AttachedFiles; -CREATE TRIGGER AttachedFileDeleted -AFTER DELETE ON AttachedFiles +CREATE TRIGGER IncrementChildCount +AFTER INSERT ON Resources FOR EACH ROW -EXECUTE PROCEDURE AttachedFileDeletedFunc(); +EXECUTE PROCEDURE UpdateChildCount(); - +CREATE TRIGGER DecrementChildCount +AFTER DELETE ON Resources +FOR EACH ROW +WHEN (OLD.parentId IS NOT NULL) +EXECUTE PROCEDURE UpdateChildCount(); -DELETE FROM GlobalProperties WHERE property IN (4); +----------- + +-- set the global properties that actually documents the DB version, revision and some of the capabilities +-- modify only the ones that have changed +DELETE FROM GlobalProperties WHERE property IN (4, 11); INSERT INTO GlobalProperties VALUES (4, 3); -- GlobalProperty_DatabasePatchLevel -
--- a/PostgreSQL/Plugins/SQL/PrepareIndex.sql Tue Feb 04 17:12:58 2025 +0100 +++ b/PostgreSQL/Plugins/SQL/PrepareIndex.sql Mon Mar 10 18:53:00 2025 +0100 @@ -630,6 +630,55 @@ $body$ LANGUAGE plpgsql; +-- -- new in rev4 + +-- This table records all resource entries whose childCount column is currently invalid +-- because of recent addition/removal of a child. +-- This way, each transaction that is adding/removing a child can add row independently +-- in this table without having to lock the parent resource row. +-- At regular interval, the DB housekeeping thread updates the childCount column of +-- resources with an entry in this table. +CREATE TABLE IF NOT EXISTS InvalidChildCounts( + id BIGINT REFERENCES Resources(internalId) ON DELETE CASCADE, + updatedAt TIMESTAMP DEFAULT NOW()); + +-- Updates the Resources.childCount column with the delta that have not been committed yet. +-- A thread will call this function at regular interval to update all pending values. +CREATE OR REPLACE FUNCTION UpdateInvalidChildCounts( + OUT updated_rows_count BIGINT +) RETURNS BIGINT AS $body$ +DECLARE + locked_resources_ids BIGINT[]; +BEGIN + + -- Lock the resources rows asap to prevent deadlocks + -- that will need to be retried + SELECT ARRAY(SELECT internalId + FROM Resources + WHERE internalId IN (SELECT DISTINCT id FROM InvalidChildCounts) + FOR UPDATE) + INTO locked_resources_ids; + + -- New rows can be added in the meantime, they won't be taken into account this time. + WITH deleted_rows AS ( + DELETE FROM InvalidChildCounts + WHERE id = ANY(locked_resources_ids) + RETURNING id + ) + + UPDATE Resources + SET childCount = (SELECT COUNT(childLevel.internalId) + FROM Resources AS childLevel + WHERE childLevel.parentId = Resources.internalId) + WHERE internalid = ANY(locked_resources_ids); + + -- Get the number of rows affected + GET DIAGNOSTICS updated_rows_count = ROW_COUNT; + +END; +$body$ LANGUAGE plpgsql; + + DROP TRIGGER IF EXISTS IncrementChildCount on Resources; DROP TRIGGER IF EXISTS DecrementChildCount on Resources; @@ -639,42 +688,20 @@ BEGIN IF TG_OP = 'INSERT' THEN IF new.parentId IS NOT NULL THEN - -- try to increment the childcount from the parent - -- note that we already have the lock on this row because the parent is locked in CreateInstance - UPDATE Resources - SET childCount = childCount + 1 - WHERE internalId = new.parentId AND childCount IS NOT NULL; - - -- this should only happen for old studies whose childCount has not yet been computed - -- note: this child has already been added so it will be counted - IF NOT FOUND THEN - UPDATE Resources - SET childCount = (SELECT COUNT(*) - FROM Resources - WHERE internalId = new.parentId) - WHERE internalId = new.parentId; - END IF; + -- mark the parent's childCount as invalid + INSERT INTO InvalidChildCounts VALUES(new.parentId); END IF; ELSIF TG_OP = 'DELETE' THEN IF old.parentId IS NOT NULL THEN - - -- Decrement the child count for the parent - -- note that we already have the lock on this row because the parent is locked in DeleteResource - UPDATE Resources - SET childCount = childCount - 1 - WHERE internalId = old.parentId AND childCount IS NOT NULL; - - -- this should only happen for old studies whose childCount has not yet been computed - -- note: this child has already been removed so it will not be counted - IF NOT FOUND THEN - UPDATE Resources - SET childCount = (SELECT COUNT(*) - FROM Resources - WHERE internalId = new.parentId) - WHERE internalId = new.parentId; - END IF; + BEGIN + -- mark the parent's childCount as invalid + INSERT INTO InvalidChildCounts VALUES(old.parentId); + EXCEPTION + -- when deleting the last child of a parent, the insert will fail (this is expected) + WHEN foreign_key_violation THEN NULL; + END; END IF; END IF;
--- a/PostgreSQL/Plugins/SQL/Upgrades/Rev2ToRev3.sql Tue Feb 04 17:12:58 2025 +0100 +++ b/PostgreSQL/Plugins/SQL/Upgrades/Rev2ToRev3.sql Mon Mar 10 18:53:00 2025 +0100 @@ -40,4 +40,4 @@ -- other changes performed in PrepareIndex.sql: - -- add ChildCount tables and triggers + -- add ChildCount triggers
--- a/PostgreSQL/Plugins/SQL/Upgrades/Rev3ToRev4.sql Tue Feb 04 17:12:58 2025 +0100 +++ b/PostgreSQL/Plugins/SQL/Upgrades/Rev3ToRev4.sql Mon Mar 10 18:53:00 2025 +0100 @@ -1,19 +1,2 @@ --- This file contains part of the changes required to upgrade from Revision 3 to Revision 4 (DB version 6) --- It actually contains only the changes that: - -- can not be executed with an idempotent statement in SQL - -- or would polute the PrepareIndex.sql --- This file is executed only if the current schema is in revision 2 and it is executed --- before PrepareIndex.sql that is idempotent. - - - -DO $body$ -BEGIN - - BEGIN - ALTER TABLE AttachedFiles ADD COLUMN customData TEXT; - EXCEPTION - WHEN duplicate_column THEN RAISE NOTICE 'column customData already exists in AttachedFiles.'; - END; - -END $body$ LANGUAGE plpgsql; +-- everything is performed in PrepareIndex.sql +SELECT 1;>>>>>>> merge rev
--- a/SQLite/Plugins/SQLiteIndex.cpp Tue Feb 04 17:12:58 2025 +0100 +++ b/SQLite/Plugins/SQLiteIndex.cpp Mon Mar 10 18:53:00 2025 +0100 @@ -206,7 +206,7 @@ SQLiteIndex::SQLiteIndex(OrthancPluginContext* context, const std::string& path) : - IndexBackend(context, false /* not read-only */), + IndexBackend(context, false /* not read-only */, false), path_(path), fast_(true) { @@ -218,7 +218,7 @@ SQLiteIndex::SQLiteIndex(OrthancPluginContext* context) : - IndexBackend(context, false /* not read-only */), + IndexBackend(context, false /* not read-only */, false), fast_(true) { }
--- a/SQLite/Plugins/SQLiteIndex.h Tue Feb 04 17:12:58 2025 +0100 +++ b/SQLite/Plugins/SQLiteIndex.h Mon Mar 10 18:53:00 2025 +0100 @@ -89,7 +89,7 @@ const Orthanc::DatabasePluginMessages::Find_Request& request) ORTHANC_OVERRIDE; #endif - virtual bool HasChildCountTable() const ORTHANC_OVERRIDE + virtual bool HasChildCountColumn() const ORTHANC_OVERRIDE { return false; }
--- a/TODO Tue Feb 04 17:12:58 2025 +0100 +++ b/TODO Mon Mar 10 18:53:00 2025 +0100 @@ -7,11 +7,6 @@ Common - Database index ----------------------- -* Performance of joins in LookupResources: Create cached statement for - LookupResources, that are grouped to search up to, say, 10 tags, - instead of recompiling for each request -* Do not log "DatabaseCannotSerialize" errors in the plugin but only - in Orthanc after all retries have been made. * Try to avoid the use of temporary tables: https://discourse.orthanc-server.org/t/image-insert-are-too-slow-databse-performance-too-poor/3820 @@ -45,16 +40,10 @@ https://discourse.orthanc-server.org/t/orthanc-container-unable-to-connect-to-specified-postgresql-database/5471 -* Have a separate "thread" to UpdateStatistics to avoid large computations ? - - * Seems Orthanc might deadlock when there are plenty of conflicting transactions: https://groups.google.com/g/orthanc-users/c/xQelEcKqL9U/m/HsvxwlkvAQAJ https://groups.google.com/g/orthanc-users/c/1bkClfZ0KBA/m/s4AlwVh3CQAJ -* use ReadCommited mode by default -* use 40 index connection count by default (not 50 because, by default, PG has 100 connections and we want at least - a few connections to remain available for sys admins if e.g, 2 Orthanc instances are running on the same server) ----- MySQL