diff --git a/autotest/ogr/ogr_pg.py b/autotest/ogr/ogr_pg.py index 520b53f0ce65..b3700c8ab97f 100755 --- a/autotest/ogr/ogr_pg.py +++ b/autotest/ogr/ogr_pg.py @@ -4858,12 +4858,14 @@ def test_ogr_pg_84(pg_ds): def test_ogr_pg_metadata(pg_ds): pg_ds = reconnect(pg_ds, update=1) + pg_ds.StartTransaction() lyr = pg_ds.CreateLayer( "test_ogr_pg_metadata", geom_type=ogr.wkbPoint, options=["OVERWRITE=YES"] ) lyr.SetMetadata({"foo": "bar"}) lyr.SetMetadataItem("bar", "baz") lyr.SetMetadataItem("DESCRIPTION", "my_desc") + pg_ds.CommitTransaction() pg_ds = reconnect(pg_ds, update=1) with pg_ds.ExecuteSQL( diff --git a/ogr/ogrsf_frmts/pg/ogr_pg.h b/ogr/ogrsf_frmts/pg/ogr_pg.h index 2bd57b53b475..014f0826602c 100644 --- a/ogr/ogrsf_frmts/pg/ogr_pg.h +++ b/ogr/ogrsf_frmts/pg/ogr_pg.h @@ -604,7 +604,7 @@ class OGRPGDataSource final : public OGRDataSource int bHavePostGIS = false; int bHaveGeography = false; - int bUserTransactionActive = false; + bool bUserTransactionActive = false; int bSavePointActive = false; int nSoftTransactionLevel = 0; @@ -638,6 +638,9 @@ class OGRPGDataSource final : public OGRDataSource int bListAllTables = false; bool m_bSkipViews = false; + bool m_bOgrSystemTablesMetadataTableExistenceTested = false; + bool m_bOgrSystemTablesMetadataTableFound = false; + void LoadTables(); CPLString osDebugLastTransactionCommand{}; @@ -740,6 +743,14 @@ class OGRPGDataSource final : public OGRDataSource int UseCopy(); void StartCopy(OGRPGTableLayer *poPGLayer); OGRErr EndCopy(); + + bool IsUserTransactionActive() + { + return bUserTransactionActive; + } + + void CreateOgrSystemTablesMetadataTableIfNeeded(); + bool HasOgrSystemTablesMetadataTable(); }; #endif /* ndef OGR_PG_H_INCLUDED */ diff --git a/ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp b/ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp index d87bc733549c..9501e96fec74 100644 --- a/ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp +++ b/ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp @@ -1309,7 +1309,8 @@ void OGRPGDataSource::LoadTables() EQUAL(pszTable, "geography_columns")) continue; - if (EQUAL(pszSchemaName, "information_schema")) + if (EQUAL(pszSchemaName, "information_schema") || + EQUAL(pszSchemaName, "ogr_system_tables")) continue; int GeomTypeFlags = 0; @@ -2606,7 +2607,7 @@ OGRErr OGRPGDataSource::StartTransaction(CPL_UNUSED int bForce) } nSoftTransactionLevel++; - bUserTransactionActive = TRUE; + bUserTransactionActive = true; /*CPLDebug("PG", "poDS=%p StartTransaction() nSoftTransactionLevel=%d", this, nSoftTransactionLevel);*/ @@ -2639,7 +2640,7 @@ OGRErr OGRPGDataSource::CommitTransaction() } nSoftTransactionLevel--; - bUserTransactionActive = FALSE; + bUserTransactionActive = false; if (bSavePointActive) { @@ -2684,7 +2685,7 @@ OGRErr OGRPGDataSource::RollbackTransaction() FlushCache(false); nSoftTransactionLevel--; - bUserTransactionActive = FALSE; + bUserTransactionActive = false; OGRErr eErr; if (bSavePointActive) @@ -3196,3 +3197,86 @@ OGRErr OGRPGDataSource::EndCopy() else return OGRERR_NONE; } + +/************************************************************************/ +/* CreateOgrSystemTablesMetadataTableIfNeeded() */ +/************************************************************************/ + +void OGRPGDataSource::CreateOgrSystemTablesMetadataTableIfNeeded() +{ + PGresult *hResult = + OGRPG_PQexec(hPGConn, "CREATE SCHEMA IF NOT EXISTS ogr_system_tables"); + OGRPGClearResult(hResult); + + hResult = OGRPG_PQexec( + hPGConn, "CREATE TABLE IF NOT EXISTS ogr_system_tables.metadata(" + "id SERIAL, " + "schema_name TEXT NOT NULL, " + "table_name TEXT NOT NULL, " + "metadata TEXT," + "UNIQUE(schema_name, table_name))"); + OGRPGClearResult(hResult); + + hResult = OGRPG_PQexec( + hPGConn, + "DROP FUNCTION IF EXISTS " + "ogr_system_tables.event_trigger_function_for_metadata() CASCADE"); + OGRPGClearResult(hResult); + + hResult = OGRPG_PQexec( + hPGConn, + "CREATE FUNCTION " + "ogr_system_tables.event_trigger_function_for_metadata()\n" + "RETURNS event_trigger LANGUAGE plpgsql AS $$\n" + "DECLARE\n" + " obj record;\n" + "BEGIN\n" + " FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects()\n" + " LOOP\n" + " IF obj.object_type = 'table' THEN\n" + " DELETE FROM ogr_system_tables.metadata m WHERE " + "m.schema_name = obj.schema_name AND m.table_name = " + "obj.object_name;\n" + " END IF;\n" + " END LOOP;\n" + "END;\n" + "$$;"); + OGRPGClearResult(hResult); + + hResult = + OGRPG_PQexec(hPGConn, "DROP EVENT TRIGGER IF EXISTS " + "ogr_system_tables_event_trigger_for_metadata"); + OGRPGClearResult(hResult); + + hResult = OGRPG_PQexec( + hPGConn, + "CREATE EVENT TRIGGER ogr_system_tables_event_trigger_for_metadata " + "ON sql_drop " + "EXECUTE FUNCTION " + "ogr_system_tables.event_trigger_function_for_metadata()"); + OGRPGClearResult(hResult); +} + +/************************************************************************/ +/* HasOgrSystemTablesMetadataTable() */ +/************************************************************************/ + +bool OGRPGDataSource::HasOgrSystemTablesMetadataTable() +{ + if (!m_bOgrSystemTablesMetadataTableExistenceTested && + CPLTestBool(CPLGetConfigOption("OGR_PG_ENABLE_METADATA", "YES"))) + { + m_bOgrSystemTablesMetadataTableExistenceTested = true; + // Check that the ogr_system_tables.metadata table exists (without + // causing errors that might abort transactions) + PGresult *hResult = OGRPG_PQexec( + hPGConn, + "SELECT c.oid FROM pg_class c " + "JOIN pg_namespace n ON c.relnamespace=n.oid " + "WHERE c.relname = 'metadata' AND n.nspname = 'ogr_system_tables'"); + m_bOgrSystemTablesMetadataTableFound = + (hResult && PQntuples(hResult) == 1 && !PQgetisnull(hResult, 0, 0)); + OGRPGClearResult(hResult); + } + return m_bOgrSystemTablesMetadataTableFound; +} diff --git a/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp b/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp index 973e8286ff43..2ac569c03c3d 100644 --- a/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp +++ b/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp @@ -248,13 +248,16 @@ void OGRPGTableLayer::LoadMetadata() return; m_bMetadataLoaded = true; + if (!poDS->HasOgrSystemTablesMetadataTable()) + return; + PGconn *hPGConn = poDS->GetPGConn(); + const std::string osSQL( CPLSPrintf("SELECT metadata FROM ogr_system_tables.metadata WHERE " "schema_name = %s AND table_name = %s", OGRPGEscapeString(hPGConn, pszSchemaName).c_str(), OGRPGEscapeString(hPGConn, pszTableName).c_str())); - CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler); auto poSqlLyr = poDS->ExecuteSQL(osSQL.c_str(), nullptr, nullptr); if (poSqlLyr) { @@ -286,8 +289,11 @@ void OGRPGTableLayer::LoadMetadata() void OGRPGTableLayer::SerializeMetadata() { - if (!m_bMetadataModified) + if (!m_bMetadataModified && + CPLTestBool(CPLGetConfigOption("OGR_PG_ENABLE_METADATA", "YES"))) + { return; + } PGconn *hPGConn = poDS->GetPGConn(); CPLXMLNode *psMD = oMDMD.Serialize(); @@ -338,66 +344,25 @@ void OGRPGTableLayer::SerializeMetadata() } } - if (psMD) + const bool bIsUserTransactionActive = poDS->IsUserTransactionActive(); { PGresult *hResult = OGRPG_PQexec( - hPGConn, "CREATE SCHEMA IF NOT EXISTS ogr_system_tables"); - OGRPGClearResult(hResult); - - hResult = OGRPG_PQexec( - hPGConn, "CREATE TABLE IF NOT EXISTS ogr_system_tables.metadata(" - "id SERIAL, " - "schema_name TEXT NOT NULL, " - "table_name TEXT NOT NULL, " - "metadata TEXT," - "UNIQUE(schema_name, table_name))"); - OGRPGClearResult(hResult); - - hResult = OGRPG_PQexec( - hPGConn, - "DROP FUNCTION IF EXISTS " - "ogr_system_tables.event_trigger_function_for_metadata() CASCADE"); - OGRPGClearResult(hResult); - - hResult = OGRPG_PQexec( - hPGConn, - "CREATE FUNCTION " - "ogr_system_tables.event_trigger_function_for_metadata()\n" - "RETURNS event_trigger LANGUAGE plpgsql AS $$\n" - "DECLARE\n" - " obj record;\n" - "BEGIN\n" - " FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects()\n" - " LOOP\n" - " IF obj.object_type = 'table' THEN\n" - " DELETE FROM ogr_system_tables.metadata m WHERE " - "m.schema_name = obj.schema_name AND m.table_name = " - "obj.object_name;\n" - " END IF;\n" - " END LOOP;\n" - "END;\n" - "$$;"); - OGRPGClearResult(hResult); - - hResult = OGRPG_PQexec(hPGConn, - "DROP EVENT TRIGGER IF EXISTS " - "ogr_system_tables_event_trigger_for_metadata"); + hPGConn, bIsUserTransactionActive + ? "SAVEPOINT ogr_system_tables_metadata_savepoint" + : "BEGIN"); OGRPGClearResult(hResult); + } - hResult = OGRPG_PQexec( - hPGConn, - "CREATE EVENT TRIGGER ogr_system_tables_event_trigger_for_metadata " - "ON sql_drop " - "EXECUTE FUNCTION " - "ogr_system_tables.event_trigger_function_for_metadata()"); - OGRPGClearResult(hResult); + if (psMD) + { + poDS->CreateOgrSystemTablesMetadataTableIfNeeded(); CPLString osCommand; osCommand.Printf("DELETE FROM ogr_system_tables.metadata WHERE " "schema_name = %s AND table_name = %s", OGRPGEscapeString(hPGConn, pszSchemaName).c_str(), OGRPGEscapeString(hPGConn, pszTableName).c_str()); - hResult = OGRPG_PQexec(hPGConn, osCommand.c_str()); + PGresult *hResult = OGRPG_PQexec(hPGConn, osCommand.c_str()); OGRPGClearResult(hResult); CPLXMLNode *psRoot = @@ -417,7 +382,7 @@ void OGRPGTableLayer::SerializeMetadata() CPLDestroyXMLNode(psRoot); CPLFree(pszXML); } - else + else if (poDS->HasOgrSystemTablesMetadataTable()) { CPLString osCommand; osCommand.Printf("DELETE FROM ogr_system_tables.metadata WHERE " @@ -428,6 +393,15 @@ void OGRPGTableLayer::SerializeMetadata() OGRPG_PQexec(hPGConn, osCommand.c_str(), false, true); OGRPGClearResult(hResult); } + + { + PGresult *hResult = OGRPG_PQexec( + hPGConn, + bIsUserTransactionActive + ? "RELEASE SAVEPOINT ogr_system_tables_metadata_savepoint" + : "COMMIT"); + OGRPGClearResult(hResult); + } } /************************************************************************/