Skip to content

Commit

Permalink
Merge pull request #9869 from OSGeo/backport-9865-to-release/3.9
Browse files Browse the repository at this point in the history 
[Backport release/3.9] PG: avoid aborting transactions when calling LoadMetadata() and ogr_system_tables.metadata doesn't exist
  • Loading branch information
@rouault
rouault authored May 6, 2024
2 parents 4986e14 + 651cae1 commit 3482a3a
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 58 deletions.
2 changes: 2 additions & 0 deletions autotest/ogr/ogr_pg.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
13 changes: 12 additions & 1 deletion ogr/ogrsf_frmts/pg/ogr_pg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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{};
Expand Down Expand Up @@ -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 */
92 changes: 88 additions & 4 deletions ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);*/
Expand Down Expand Up @@ -2639,7 +2640,7 @@ OGRErr OGRPGDataSource::CommitTransaction()
}

nSoftTransactionLevel--;
bUserTransactionActive = FALSE;
bUserTransactionActive = false;

if (bSavePointActive)
{
Expand Down Expand Up @@ -2684,7 +2685,7 @@ OGRErr OGRPGDataSource::RollbackTransaction()
FlushCache(false);

nSoftTransactionLevel--;
bUserTransactionActive = FALSE;
bUserTransactionActive = false;

OGRErr eErr;
if (bSavePointActive)
Expand Down Expand Up @@ -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;
}
80 changes: 27 additions & 53 deletions ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 =
Expand All @@ -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 "
Expand All @@ -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);
}
}

/************************************************************************/
Expand Down

0 comments on commit 3482a3a

Please sign in to comment.