Skip to content

Commit 3482a3a

Browse files
authored
Merge pull request #9869 from OSGeo/backport-9865-to-release/3.9
[Backport release/3.9] PG: avoid aborting transactions when calling LoadMetadata() and ogr_system_tables.metadata doesn't exist
2 parents 4986e14 + 651cae1 commit 3482a3a

File tree

4 files changed

+129
-58
lines changed

4 files changed

+129
-58
lines changed

autotest/ogr/ogr_pg.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4858,12 +4858,14 @@ def test_ogr_pg_84(pg_ds):
48584858
def test_ogr_pg_metadata(pg_ds):
48594859

48604860
pg_ds = reconnect(pg_ds, update=1)
4861+
pg_ds.StartTransaction()
48614862
lyr = pg_ds.CreateLayer(
48624863
"test_ogr_pg_metadata", geom_type=ogr.wkbPoint, options=["OVERWRITE=YES"]
48634864
)
48644865
lyr.SetMetadata({"foo": "bar"})
48654866
lyr.SetMetadataItem("bar", "baz")
48664867
lyr.SetMetadataItem("DESCRIPTION", "my_desc")
4868+
pg_ds.CommitTransaction()
48674869

48684870
pg_ds = reconnect(pg_ds, update=1)
48694871
with pg_ds.ExecuteSQL(

ogr/ogrsf_frmts/pg/ogr_pg.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ class OGRPGDataSource final : public OGRDataSource
604604
int bHavePostGIS = false;
605605
int bHaveGeography = false;
606606

607-
int bUserTransactionActive = false;
607+
bool bUserTransactionActive = false;
608608
int bSavePointActive = false;
609609
int nSoftTransactionLevel = 0;
610610

@@ -638,6 +638,9 @@ class OGRPGDataSource final : public OGRDataSource
638638
int bListAllTables = false;
639639
bool m_bSkipViews = false;
640640

641+
bool m_bOgrSystemTablesMetadataTableExistenceTested = false;
642+
bool m_bOgrSystemTablesMetadataTableFound = false;
643+
641644
void LoadTables();
642645

643646
CPLString osDebugLastTransactionCommand{};
@@ -740,6 +743,14 @@ class OGRPGDataSource final : public OGRDataSource
740743
int UseCopy();
741744
void StartCopy(OGRPGTableLayer *poPGLayer);
742745
OGRErr EndCopy();
746+
747+
bool IsUserTransactionActive()
748+
{
749+
return bUserTransactionActive;
750+
}
751+
752+
void CreateOgrSystemTablesMetadataTableIfNeeded();
753+
bool HasOgrSystemTablesMetadataTable();
743754
};
744755

745756
#endif /* ndef OGR_PG_H_INCLUDED */

ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,8 @@ void OGRPGDataSource::LoadTables()
13091309
EQUAL(pszTable, "geography_columns"))
13101310
continue;
13111311

1312-
if (EQUAL(pszSchemaName, "information_schema"))
1312+
if (EQUAL(pszSchemaName, "information_schema") ||
1313+
EQUAL(pszSchemaName, "ogr_system_tables"))
13131314
continue;
13141315

13151316
int GeomTypeFlags = 0;
@@ -2606,7 +2607,7 @@ OGRErr OGRPGDataSource::StartTransaction(CPL_UNUSED int bForce)
26062607
}
26072608

26082609
nSoftTransactionLevel++;
2609-
bUserTransactionActive = TRUE;
2610+
bUserTransactionActive = true;
26102611

26112612
/*CPLDebug("PG", "poDS=%p StartTransaction() nSoftTransactionLevel=%d",
26122613
this, nSoftTransactionLevel);*/
@@ -2639,7 +2640,7 @@ OGRErr OGRPGDataSource::CommitTransaction()
26392640
}
26402641

26412642
nSoftTransactionLevel--;
2642-
bUserTransactionActive = FALSE;
2643+
bUserTransactionActive = false;
26432644

26442645
if (bSavePointActive)
26452646
{
@@ -2684,7 +2685,7 @@ OGRErr OGRPGDataSource::RollbackTransaction()
26842685
FlushCache(false);
26852686

26862687
nSoftTransactionLevel--;
2687-
bUserTransactionActive = FALSE;
2688+
bUserTransactionActive = false;
26882689

26892690
OGRErr eErr;
26902691
if (bSavePointActive)
@@ -3196,3 +3197,86 @@ OGRErr OGRPGDataSource::EndCopy()
31963197
else
31973198
return OGRERR_NONE;
31983199
}
3200+
3201+
/************************************************************************/
3202+
/* CreateOgrSystemTablesMetadataTableIfNeeded() */
3203+
/************************************************************************/
3204+
3205+
void OGRPGDataSource::CreateOgrSystemTablesMetadataTableIfNeeded()
3206+
{
3207+
PGresult *hResult =
3208+
OGRPG_PQexec(hPGConn, "CREATE SCHEMA IF NOT EXISTS ogr_system_tables");
3209+
OGRPGClearResult(hResult);
3210+
3211+
hResult = OGRPG_PQexec(
3212+
hPGConn, "CREATE TABLE IF NOT EXISTS ogr_system_tables.metadata("
3213+
"id SERIAL, "
3214+
"schema_name TEXT NOT NULL, "
3215+
"table_name TEXT NOT NULL, "
3216+
"metadata TEXT,"
3217+
"UNIQUE(schema_name, table_name))");
3218+
OGRPGClearResult(hResult);
3219+
3220+
hResult = OGRPG_PQexec(
3221+
hPGConn,
3222+
"DROP FUNCTION IF EXISTS "
3223+
"ogr_system_tables.event_trigger_function_for_metadata() CASCADE");
3224+
OGRPGClearResult(hResult);
3225+
3226+
hResult = OGRPG_PQexec(
3227+
hPGConn,
3228+
"CREATE FUNCTION "
3229+
"ogr_system_tables.event_trigger_function_for_metadata()\n"
3230+
"RETURNS event_trigger LANGUAGE plpgsql AS $$\n"
3231+
"DECLARE\n"
3232+
" obj record;\n"
3233+
"BEGIN\n"
3234+
" FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects()\n"
3235+
" LOOP\n"
3236+
" IF obj.object_type = 'table' THEN\n"
3237+
" DELETE FROM ogr_system_tables.metadata m WHERE "
3238+
"m.schema_name = obj.schema_name AND m.table_name = "
3239+
"obj.object_name;\n"
3240+
" END IF;\n"
3241+
" END LOOP;\n"
3242+
"END;\n"
3243+
"$$;");
3244+
OGRPGClearResult(hResult);
3245+
3246+
hResult =
3247+
OGRPG_PQexec(hPGConn, "DROP EVENT TRIGGER IF EXISTS "
3248+
"ogr_system_tables_event_trigger_for_metadata");
3249+
OGRPGClearResult(hResult);
3250+
3251+
hResult = OGRPG_PQexec(
3252+
hPGConn,
3253+
"CREATE EVENT TRIGGER ogr_system_tables_event_trigger_for_metadata "
3254+
"ON sql_drop "
3255+
"EXECUTE FUNCTION "
3256+
"ogr_system_tables.event_trigger_function_for_metadata()");
3257+
OGRPGClearResult(hResult);
3258+
}
3259+
3260+
/************************************************************************/
3261+
/* HasOgrSystemTablesMetadataTable() */
3262+
/************************************************************************/
3263+
3264+
bool OGRPGDataSource::HasOgrSystemTablesMetadataTable()
3265+
{
3266+
if (!m_bOgrSystemTablesMetadataTableExistenceTested &&
3267+
CPLTestBool(CPLGetConfigOption("OGR_PG_ENABLE_METADATA", "YES")))
3268+
{
3269+
m_bOgrSystemTablesMetadataTableExistenceTested = true;
3270+
// Check that the ogr_system_tables.metadata table exists (without
3271+
// causing errors that might abort transactions)
3272+
PGresult *hResult = OGRPG_PQexec(
3273+
hPGConn,
3274+
"SELECT c.oid FROM pg_class c "
3275+
"JOIN pg_namespace n ON c.relnamespace=n.oid "
3276+
"WHERE c.relname = 'metadata' AND n.nspname = 'ogr_system_tables'");
3277+
m_bOgrSystemTablesMetadataTableFound =
3278+
(hResult && PQntuples(hResult) == 1 && !PQgetisnull(hResult, 0, 0));
3279+
OGRPGClearResult(hResult);
3280+
}
3281+
return m_bOgrSystemTablesMetadataTableFound;
3282+
}

ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp

Lines changed: 27 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -248,13 +248,16 @@ void OGRPGTableLayer::LoadMetadata()
248248
return;
249249
m_bMetadataLoaded = true;
250250

251+
if (!poDS->HasOgrSystemTablesMetadataTable())
252+
return;
253+
251254
PGconn *hPGConn = poDS->GetPGConn();
255+
252256
const std::string osSQL(
253257
CPLSPrintf("SELECT metadata FROM ogr_system_tables.metadata WHERE "
254258
"schema_name = %s AND table_name = %s",
255259
OGRPGEscapeString(hPGConn, pszSchemaName).c_str(),
256260
OGRPGEscapeString(hPGConn, pszTableName).c_str()));
257-
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
258261
auto poSqlLyr = poDS->ExecuteSQL(osSQL.c_str(), nullptr, nullptr);
259262
if (poSqlLyr)
260263
{
@@ -286,8 +289,11 @@ void OGRPGTableLayer::LoadMetadata()
286289

287290
void OGRPGTableLayer::SerializeMetadata()
288291
{
289-
if (!m_bMetadataModified)
292+
if (!m_bMetadataModified &&
293+
CPLTestBool(CPLGetConfigOption("OGR_PG_ENABLE_METADATA", "YES")))
294+
{
290295
return;
296+
}
291297

292298
PGconn *hPGConn = poDS->GetPGConn();
293299
CPLXMLNode *psMD = oMDMD.Serialize();
@@ -338,66 +344,25 @@ void OGRPGTableLayer::SerializeMetadata()
338344
}
339345
}
340346

341-
if (psMD)
347+
const bool bIsUserTransactionActive = poDS->IsUserTransactionActive();
342348
{
343349
PGresult *hResult = OGRPG_PQexec(
344-
hPGConn, "CREATE SCHEMA IF NOT EXISTS ogr_system_tables");
345-
OGRPGClearResult(hResult);
346-
347-
hResult = OGRPG_PQexec(
348-
hPGConn, "CREATE TABLE IF NOT EXISTS ogr_system_tables.metadata("
349-
"id SERIAL, "
350-
"schema_name TEXT NOT NULL, "
351-
"table_name TEXT NOT NULL, "
352-
"metadata TEXT,"
353-
"UNIQUE(schema_name, table_name))");
354-
OGRPGClearResult(hResult);
355-
356-
hResult = OGRPG_PQexec(
357-
hPGConn,
358-
"DROP FUNCTION IF EXISTS "
359-
"ogr_system_tables.event_trigger_function_for_metadata() CASCADE");
360-
OGRPGClearResult(hResult);
361-
362-
hResult = OGRPG_PQexec(
363-
hPGConn,
364-
"CREATE FUNCTION "
365-
"ogr_system_tables.event_trigger_function_for_metadata()\n"
366-
"RETURNS event_trigger LANGUAGE plpgsql AS $$\n"
367-
"DECLARE\n"
368-
" obj record;\n"
369-
"BEGIN\n"
370-
" FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects()\n"
371-
" LOOP\n"
372-
" IF obj.object_type = 'table' THEN\n"
373-
" DELETE FROM ogr_system_tables.metadata m WHERE "
374-
"m.schema_name = obj.schema_name AND m.table_name = "
375-
"obj.object_name;\n"
376-
" END IF;\n"
377-
" END LOOP;\n"
378-
"END;\n"
379-
"$$;");
380-
OGRPGClearResult(hResult);
381-
382-
hResult = OGRPG_PQexec(hPGConn,
383-
"DROP EVENT TRIGGER IF EXISTS "
384-
"ogr_system_tables_event_trigger_for_metadata");
350+
hPGConn, bIsUserTransactionActive
351+
? "SAVEPOINT ogr_system_tables_metadata_savepoint"
352+
: "BEGIN");
385353
OGRPGClearResult(hResult);
354+
}
386355

387-
hResult = OGRPG_PQexec(
388-
hPGConn,
389-
"CREATE EVENT TRIGGER ogr_system_tables_event_trigger_for_metadata "
390-
"ON sql_drop "
391-
"EXECUTE FUNCTION "
392-
"ogr_system_tables.event_trigger_function_for_metadata()");
393-
OGRPGClearResult(hResult);
356+
if (psMD)
357+
{
358+
poDS->CreateOgrSystemTablesMetadataTableIfNeeded();
394359

395360
CPLString osCommand;
396361
osCommand.Printf("DELETE FROM ogr_system_tables.metadata WHERE "
397362
"schema_name = %s AND table_name = %s",
398363
OGRPGEscapeString(hPGConn, pszSchemaName).c_str(),
399364
OGRPGEscapeString(hPGConn, pszTableName).c_str());
400-
hResult = OGRPG_PQexec(hPGConn, osCommand.c_str());
365+
PGresult *hResult = OGRPG_PQexec(hPGConn, osCommand.c_str());
401366
OGRPGClearResult(hResult);
402367

403368
CPLXMLNode *psRoot =
@@ -417,7 +382,7 @@ void OGRPGTableLayer::SerializeMetadata()
417382
CPLDestroyXMLNode(psRoot);
418383
CPLFree(pszXML);
419384
}
420-
else
385+
else if (poDS->HasOgrSystemTablesMetadataTable())
421386
{
422387
CPLString osCommand;
423388
osCommand.Printf("DELETE FROM ogr_system_tables.metadata WHERE "
@@ -428,6 +393,15 @@ void OGRPGTableLayer::SerializeMetadata()
428393
OGRPG_PQexec(hPGConn, osCommand.c_str(), false, true);
429394
OGRPGClearResult(hResult);
430395
}
396+
397+
{
398+
PGresult *hResult = OGRPG_PQexec(
399+
hPGConn,
400+
bIsUserTransactionActive
401+
? "RELEASE SAVEPOINT ogr_system_tables_metadata_savepoint"
402+
: "COMMIT");
403+
OGRPGClearResult(hResult);
404+
}
431405
}
432406

433407
/************************************************************************/

0 commit comments

Comments
 (0)