Mercurial > hg > orthanc-databases
changeset 266:cc7af42d4f23
Store revisions for metadata and attachments in PostgreSQL
| author | Sebastien Jodogne <s.jodogne@gmail.com> |
|---|---|
| date | Tue, 20 Apr 2021 17:41:44 +0200 |
| parents | cd73e34d5411 |
| children | ece4663dedde |
| files | Framework/Plugins/IndexBackend.cpp Framework/Plugins/IndexUnitTests.h Framework/PostgreSQL/PostgreSQLDatabase.cpp Framework/PostgreSQL/PostgreSQLDatabase.h PostgreSQL/NEWS PostgreSQL/Plugins/PostgreSQLIndex.cpp PostgreSQL/Plugins/PostgreSQLIndex.h PostgreSQL/UnitTests/PostgreSQLTests.cpp TODO |
| diffstat | 9 files changed, 159 insertions(+), 63 deletions(-) [+] |
line wrap: on
line diff
--- a/Framework/Plugins/IndexBackend.cpp Tue Apr 20 16:14:04 2021 +0200 +++ b/Framework/Plugins/IndexBackend.cpp Tue Apr 20 17:41:44 2021 +0200 @@ -1037,7 +1037,17 @@ if (ExecuteLookupAttachment(statement, output, id, contentType)) { - revision = statement.ReadInteger64(6); + if (statement.GetResultField(6).GetType() == ValueType_Null) + { + // "NULL" can happen with a database created by PostgreSQL + // plugin <= 3.3 (because of "ALTER TABLE AttachedFiles") + revision = 0; + } + else + { + revision = statement.ReadInteger64(6); + } + return true; } else @@ -1259,25 +1269,18 @@ { std::unique_ptr<DatabaseManager::CachedStatement> statement; - switch (manager.GetDialect()) + if (HasRevisionsSupport()) { - case Dialect_MySQL: - case Dialect_PostgreSQL: - statement.reset(new DatabaseManager::CachedStatement( - STATEMENT_FROM_HERE, manager, - "SELECT value FROM Metadata WHERE id=${id} and type=${type}")); - break; - - case Dialect_SQLite: - statement.reset(new DatabaseManager::CachedStatement( - STATEMENT_FROM_HERE, manager, - "SELECT value, revision FROM Metadata WHERE id=${id} and type=${type}")); - break; - - default: - throw Orthanc::OrthancException(Orthanc::ErrorCode_NotImplemented); + statement.reset(new DatabaseManager::CachedStatement( + STATEMENT_FROM_HERE, manager, + "SELECT value, revision FROM Metadata WHERE id=${id} and type=${type}")); } - + else + { + statement.reset(new DatabaseManager::CachedStatement( + STATEMENT_FROM_HERE, manager, + "SELECT value FROM Metadata WHERE id=${id} and type=${type}")); + } statement->SetReadOnly(true); statement->SetParameterType("id", ValueType_Integer64); @@ -1297,13 +1300,22 @@ { target = statement->ReadString(0); - if (manager.GetDialect() == Dialect_SQLite) + if (HasRevisionsSupport()) { - revision = statement->ReadInteger64(1); + if (statement->GetResultField(1).GetType() == ValueType_Null) + { + // "NULL" can happen with a database created by PostgreSQL + // plugin <= 3.3 (because of "ALTER TABLE AttachedFiles") + revision = 0; + } + else + { + revision = statement->ReadInteger64(1); + } } else { - revision = 0; // TODO - REVISIONS + revision = 0; // No support for revisions } return true; @@ -1581,6 +1593,23 @@ ExecuteSetTag(statement, id, group, element, value); } + + static void ExecuteSetMetadata(DatabaseManager::CachedStatement& statement, + Dictionary& args, + int64_t id, + int32_t metadataType, + const char* value) + { + statement.SetParameterType("id", ValueType_Integer64); + statement.SetParameterType("type", ValueType_Integer64); + statement.SetParameterType("value", ValueType_Utf8String); + + args.SetIntegerValue("id", id); + args.SetIntegerValue("type", metadataType); + args.SetUtf8Value("value", value); + + statement.Execute(args); + } void IndexBackend::SetMetadata(DatabaseManager& manager, int64_t id, @@ -1590,26 +1619,19 @@ { if (manager.GetDialect() == Dialect_SQLite) { + assert(HasRevisionsSupport()); DatabaseManager::CachedStatement statement( STATEMENT_FROM_HERE, manager, "INSERT OR REPLACE INTO Metadata VALUES (${id}, ${type}, ${value}, ${revision})"); - statement.SetParameterType("id", ValueType_Integer64); - statement.SetParameterType("type", ValueType_Integer64); - statement.SetParameterType("value", ValueType_Utf8String); + Dictionary args; statement.SetParameterType("revision", ValueType_Integer64); - - Dictionary args; - args.SetIntegerValue("id", id); - args.SetIntegerValue("type", metadataType); - args.SetUtf8Value("value", value); args.SetIntegerValue("revision", revision); - - statement.Execute(args); + + ExecuteSetMetadata(statement, args, id, metadataType, value); } else { - // TODO - REVISIONS { DatabaseManager::CachedStatement statement( STATEMENT_FROM_HERE, manager, @@ -1625,21 +1647,26 @@ statement.Execute(args); } + if (HasRevisionsSupport()) + { + DatabaseManager::CachedStatement statement( + STATEMENT_FROM_HERE, manager, + "INSERT INTO Metadata VALUES (${id}, ${type}, ${value}, ${revision})"); + + Dictionary args; + statement.SetParameterType("revision", ValueType_Integer64); + args.SetIntegerValue("revision", revision); + + ExecuteSetMetadata(statement, args, id, metadataType, value); + } + else { DatabaseManager::CachedStatement statement( STATEMENT_FROM_HERE, manager, "INSERT INTO Metadata VALUES (${id}, ${type}, ${value})"); - statement.SetParameterType("id", ValueType_Integer64); - statement.SetParameterType("type", ValueType_Integer64); - statement.SetParameterType("value", ValueType_Utf8String); - Dictionary args; - args.SetIntegerValue("id", id); - args.SetIntegerValue("type", metadataType); - args.SetUtf8Value("value", value); - - statement.Execute(args); + ExecuteSetMetadata(statement, args, id, metadataType, value); } } } @@ -2065,6 +2092,7 @@ #if ORTHANC_PLUGINS_HAS_DATABASE_CONSTRAINT == 1 static void ExecuteSetResourcesContentMetadata( DatabaseManager& manager, + bool hasRevisionsSupport, uint32_t count, const OrthancPluginResourcesContentMetadata* metadata) { @@ -2079,9 +2107,9 @@ args.SetUtf8Value(name, metadata[i].value); std::string revisionSuffix; - if (manager.GetDialect() == Dialect_SQLite) + if (hasRevisionsSupport) { - revisionSuffix = ", 0"; // TODO - REVISIONS + revisionSuffix = ", 0"; } std::string insert = ("(" + boost::lexical_cast<std::string>(metadata[i].resource) + ", " + @@ -2157,7 +2185,7 @@ ExecuteSetResourcesContentTags(manager, "MainDicomTags", "t", countMainDicomTags, mainDicomTags); - ExecuteSetResourcesContentMetadata(manager, countMetadata, metadata); + ExecuteSetResourcesContentMetadata(manager, HasRevisionsSupport(), countMetadata, metadata); } #endif
--- a/Framework/Plugins/IndexUnitTests.h Tue Apr 20 16:14:04 2021 +0200 +++ b/Framework/Plugins/IndexUnitTests.h Tue Apr 20 17:41:44 2021 +0200 @@ -33,6 +33,17 @@ #include <list> +#if ORTHANC_ENABLE_POSTGRESQL == 1 +# define HAS_REVISIONS 1 +#elif ORTHANC_ENABLE_MYSQL == 1 +# define HAS_REVISIONS 0 +#elif ORTHANC_ENABLE_SQLITE == 1 +# define HAS_REVISIONS 1 +#else +# error Unknown database backend +#endif + + namespace Orthanc { /** @@ -277,8 +288,8 @@ ASSERT_TRUE(db.LookupMetadata(s, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); ASSERT_EQ("update2", s); -#if ORTHANC_ENABLE_SQLITE == 1 - ASSERT_EQ(43, revision); // Only SQLite implements revisions so far +#if HAS_REVISIONS == 1 + ASSERT_EQ(43, revision); #else ASSERT_EQ(0, revision); #endif @@ -287,8 +298,8 @@ ASSERT_TRUE(db.LookupMetadata(s, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); ASSERT_EQ("update", s); -#if ORTHANC_ENABLE_SQLITE == 1 - ASSERT_EQ(44, revision); // Only SQLite implements revisions so far +#if HAS_REVISIONS == 1 + ASSERT_EQ(44, revision); #else ASSERT_EQ(0, revision); #endif @@ -302,8 +313,8 @@ ASSERT_TRUE(db.LookupMetadata(mdd, revision, *manager, a, Orthanc::MetadataType_ModifiedFrom)); ASSERT_EQ("modified", mdd); -#if ORTHANC_ENABLE_SQLITE == 1 - ASSERT_EQ(42, revision); // Only SQLite implements revisions so far +#if HAS_REVISIONS == 1 + ASSERT_EQ(42, revision); #else ASSERT_EQ(0, revision); #endif @@ -311,8 +322,8 @@ ASSERT_TRUE(db.LookupMetadata(mdd, revision, *manager, a, Orthanc::MetadataType_LastUpdate)); ASSERT_EQ("update", mdd); -#if ORTHANC_ENABLE_SQLITE == 1 - ASSERT_EQ(44, revision); // Only SQLite implements revisions so far +#if HAS_REVISIONS == 1 + ASSERT_EQ(44, revision); #else ASSERT_EQ(0, revision); #endif @@ -376,8 +387,8 @@ expectedAttachment->compressedHash = "md5_1"; ASSERT_TRUE(db.LookupAttachment(*output, revision, *manager, a, Orthanc::FileContentType_Dicom)); -#if ORTHANC_ENABLE_SQLITE == 1 - ASSERT_EQ(42, revision); // Only SQLite implements revisions so far +#if HAS_REVISIONS == 1 + ASSERT_EQ(42, revision); #else ASSERT_EQ(0, revision); #endif @@ -393,8 +404,8 @@ revision = -1; ASSERT_TRUE(db.LookupAttachment(*output, revision, *manager, a, Orthanc::FileContentType_DicomAsJson)); -#if ORTHANC_ENABLE_SQLITE == 1 - ASSERT_EQ(43, revision); // Only SQLite implements revisions so far +#if HAS_REVISIONS == 1 + ASSERT_EQ(43, revision); #else ASSERT_EQ(0, revision); #endif
--- a/Framework/PostgreSQL/PostgreSQLDatabase.cpp Tue Apr 20 16:14:04 2021 +0200 +++ b/Framework/PostgreSQL/PostgreSQLDatabase.cpp Tue Apr 20 17:41:44 2021 +0200 @@ -30,6 +30,7 @@ #include <Logging.h> #include <OrthancException.h> +#include <Toolbox.h> #include <boost/lexical_cast.hpp> #include <boost/thread.hpp> @@ -184,10 +185,10 @@ } - bool PostgreSQLDatabase::DoesTableExist(const char* name) + bool PostgreSQLDatabase::DoesTableExist(const std::string& name) { - std::string lower(name); - std::transform(lower.begin(), lower.end(), lower.begin(), tolower); + std::string lower; + Orthanc::Toolbox::ToLowerCase(lower, name); // http://stackoverflow.com/a/24089729/881731 @@ -205,6 +206,30 @@ } + bool PostgreSQLDatabase::DoesColumnExist(const std::string& tableName, + const std::string& columnName) + { + std::string lowerTable, lowerColumn; + Orthanc::Toolbox::ToLowerCase(lowerTable, tableName); + Orthanc::Toolbox::ToLowerCase(lowerColumn, columnName); + + PostgreSQLStatement statement(*this, + "SELECT 1 FROM information_schema.columns " + "WHERE table_schema=$1 AND table_name=$2 AND column_name=$3"); + + statement.DeclareInputString(0); + statement.DeclareInputString(1); + statement.DeclareInputString(2); + + statement.BindString(0, "public" /* schema */); + statement.BindString(1, lowerTable); + statement.BindString(2, lowerColumn); + + PostgreSQLResult result(statement); + return !result.IsDone(); + } + + void PostgreSQLDatabase::ClearAll() { PostgreSQLTransaction transaction(*this, TransactionType_ReadWrite);
--- a/Framework/PostgreSQL/PostgreSQLDatabase.h Tue Apr 20 16:14:04 2021 +0200 +++ b/Framework/PostgreSQL/PostgreSQLDatabase.h Tue Apr 20 17:41:44 2021 +0200 @@ -66,7 +66,10 @@ void ExecuteMultiLines(const std::string& sql); - bool DoesTableExist(const char* name); + bool DoesTableExist(const std::string& name); + + bool DoesColumnExist(const std::string& tableName, + const std::string& columnName); void ClearAll(); // Only for unit tests!
--- a/PostgreSQL/NEWS Tue Apr 20 16:14:04 2021 +0200 +++ b/PostgreSQL/NEWS Tue Apr 20 17:41:44 2021 +0200 @@ -3,6 +3,7 @@ * New option "IndexConnectionsCount" to control how many simultaneous connections to the PostgreSQL database are used by the index plugin +* Support of revisions for metadata and attachments * Support of multiple readers/writers, by handling retries from Orthanc SDK 1.9.2 * Support of range reads for the storage area, from Orthanc SDK 1.9.0
--- a/PostgreSQL/Plugins/PostgreSQLIndex.cpp Tue Apr 20 16:14:04 2021 +0200 +++ b/PostgreSQL/Plugins/PostgreSQLIndex.cpp Tue Apr 20 17:41:44 2021 +0200 @@ -276,6 +276,24 @@ "property INTEGER, value TEXT, PRIMARY KEY(server, property))"); } + /** + * PostgreSQL 9.5: "Adding a column with a default requires + * updating each row of the table (to store the new column + * value). However, if no default is specified, PostgreSQL is + * able to avoid the physical update." => We set no default + * for performance (older entries will be NULL) + * https://www.postgresql.org/docs/9.5/ddl-alter.html + **/ + if (!db.DoesColumnExist("Metadata", "revision")) + { + t.GetDatabaseTransaction().ExecuteMultiLines("ALTER TABLE Metadata ADD COLUMN revision INTEGER"); + } + + if (!db.DoesColumnExist("AttachedFiles", "revision")) + { + t.GetDatabaseTransaction().ExecuteMultiLines("ALTER TABLE AttachedFiles ADD COLUMN revision INTEGER"); + } + t.Commit(); } }
--- a/PostgreSQL/Plugins/PostgreSQLIndex.h Tue Apr 20 16:14:04 2021 +0200 +++ b/PostgreSQL/Plugins/PostgreSQLIndex.h Tue Apr 20 17:41:44 2021 +0200 @@ -47,7 +47,7 @@ virtual bool HasRevisionsSupport() const ORTHANC_OVERRIDE { - return false; // TODO - REVISIONS + return true; } virtual int64_t CreateResource(DatabaseManager& manager,
--- a/PostgreSQL/UnitTests/PostgreSQLTests.cpp Tue Apr 20 16:14:04 2021 +0200 +++ b/PostgreSQL/UnitTests/PostgreSQLTests.cpp Tue Apr 20 17:41:44 2021 +0200 @@ -86,9 +86,18 @@ std::unique_ptr<PostgreSQLDatabase> pg(CreateTestDatabase()); ASSERT_FALSE(pg->DoesTableExist("Test")); + ASSERT_FALSE(pg->DoesColumnExist("Test", "value")); + ASSERT_FALSE(pg->DoesTableExist("TEST")); + ASSERT_FALSE(pg->DoesTableExist("test")); pg->ExecuteMultiLines("CREATE TABLE Test(name INTEGER, value BIGINT)"); ASSERT_TRUE(pg->DoesTableExist("Test")); + ASSERT_TRUE(pg->DoesTableExist("TEST")); + ASSERT_TRUE(pg->DoesTableExist("test")); + ASSERT_TRUE(pg->DoesColumnExist("Test", "Value")); + ASSERT_TRUE(pg->DoesColumnExist("TEST", "VALUE")); + ASSERT_TRUE(pg->DoesColumnExist("test", "value")); + PostgreSQLStatement s(*pg, "INSERT INTO Test VALUES ($1,$2)"); s.DeclareInputInteger(0); s.DeclareInputInteger64(1);
--- a/TODO Tue Apr 20 16:14:04 2021 +0200 +++ b/TODO Tue Apr 20 17:41:44 2021 +0200 @@ -7,8 +7,6 @@ Common - Database index ----------------------- -* Store revisions for metadata and attachments in PostgreSQL and MySQL - * 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 @@ -30,6 +28,9 @@ MySQL ----- +* Store revisions for metadata and attachments in MySQL (this is + already implemented in PostgreSQL) + * MySQL performance => implement GlobalProperty_GetTotalSizeIsFast: https://groups.google.com/d/msg/orthanc-users/kSR4a110zDo/D7e4ITR8BwAJ