Merge pull request #11413 from OSGeo/backport-11389-to-release/3.10

[Backport release/3.10] PG: avoid error when the original search_path contains something like '"", something_else'
OSGeo · Dec 2, 2024 · 0bc2d8c · 0bc2d8c
2 parents ef1a36a + 94e4a6d
commit 0bc2d8c
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 24 deletions.
diff --git a/autotest/ogr/ogr_pg.py b/autotest/ogr/ogr_pg.py
@@ -6204,6 +6204,7 @@ def test_ogr_pg_ogr2ogr_with_multiple_dotted_table_name(pg_ds):
 
 ###############################################################################
 # Test scenario of https://lists.osgeo.org/pipermail/gdal-dev/2024-October/059608.html
+# and bugfix of https://github.com/OSGeo/gdal/issues/11386
 
 
 @only_without_postgis
@@ -6220,14 +6221,33 @@ def test_ogr_pg_empty_search_path(pg_ds):
     with pg_ds.ExecuteSQL("SELECT CURRENT_USER") as lyr:
         f = lyr.GetNextFeature()
         current_user = f.GetField(0)
+
     pg_ds.ExecuteSQL(f"ALTER ROLE {current_user} SET search_path = ''")
     try:
         ds = reconnect(pg_ds, update=1)
 
         with ds.ExecuteSQL("SHOW search_path") as sql_lyr:
             f = sql_lyr.GetNextFeature()
             new_search_path = f.GetField(0)
-            assert new_search_path == "test_ogr_pg_empty_search_path_no_postgis, public"
+            assert (
+                new_search_path
+                == 'test_ogr_pg_empty_search_path_no_postgis, "", public'
+            )
+
+    finally:
+        ds.ExecuteSQL(f"ALTER ROLE {current_user} SET search_path = {old_search_path}")
+
+    pg_ds.ExecuteSQL(f"ALTER ROLE {current_user} SET search_path = '', '$user'")
+    try:
+        ds = reconnect(pg_ds, update=1)
+
+        with ds.ExecuteSQL("SHOW search_path") as sql_lyr:
+            f = sql_lyr.GetNextFeature()
+            new_search_path = f.GetField(0)
+            assert (
+                new_search_path
+                == 'test_ogr_pg_empty_search_path_no_postgis, "", "$user", public'
+            )
 
     finally:
         ds.ExecuteSQL(f"ALTER ROLE {current_user} SET search_path = {old_search_path}")
diff --git a/ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp b/ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp
@@ -649,12 +649,16 @@ int OGRPGDataSource::Open(const char *pszNewName, int bUpdate, int bTestOpen,
          osSearchPath.find(osPostgisSchema) == std::string::npos))
     {
         std::string osNewSearchPath;
-        if (osActiveSchema != "public")
+        if (!osActiveSchema.empty() && osActiveSchema != "public")
         {
             osNewSearchPath +=
                 OGRPGEscapeString(hPGConn, osActiveSchema.c_str());
         }
-        if (!osSearchPath.empty() && osSearchPath != "\"\"")
+        // SHOW search_path reports "", but SET search_path doesn't accept it!
+        // It must be transformed to ''
+        if (STARTS_WITH(osSearchPath.c_str(), "\"\""))
+            osSearchPath = "''" + osSearchPath.substr(2);
+        if (!osSearchPath.empty())
         {
             if (!osNewSearchPath.empty())
                 osNewSearchPath += ',';
@@ -668,34 +672,38 @@ int OGRPGDataSource::Open(const char *pszNewName, int bUpdate, int bTestOpen,
             osNewSearchPath +=
                 OGRPGEscapeString(hPGConn, osPostgisSchema.c_str());
         }
-        CPLDebug("PG", "Modifying search_path from %s to %s",
-                 osSearchPath.c_str(), osNewSearchPath.c_str());
+        if (osNewSearchPath != osSearchPath)
+        {
+            CPLDebug("PG", "Modifying search_path from %s to %s",
+                     osSearchPath.c_str(), osNewSearchPath.c_str());
 
-        std::string osCommand = "SET search_path=" + osNewSearchPath;
-        PGresult *hResult = OGRPG_PQexec(hPGConn, osCommand.c_str());
+            std::string osCommand = "SET search_path=" + osNewSearchPath;
+            PGresult *hResult = OGRPG_PQexec(hPGConn, osCommand.c_str());
 
-        if (!hResult || PQresultStatus(hResult) != PGRES_COMMAND_OK)
-        {
-            OGRPGClearResult(hResult);
-            CPLDebug("PG", "Command \"%s\" failed. Trying without 'public'.",
-                     osCommand.c_str());
-            osCommand = CPLSPrintf(
-                "SET search_path=%s",
-                OGRPGEscapeString(hPGConn, osActiveSchema.c_str()).c_str());
-            PGresult *hResult2 = OGRPG_PQexec(hPGConn, osCommand.c_str());
-
-            if (!hResult2 || PQresultStatus(hResult2) != PGRES_COMMAND_OK)
+            if (!hResult || PQresultStatus(hResult) != PGRES_COMMAND_OK)
             {
-                OGRPGClearResult(hResult2);
+                OGRPGClearResult(hResult);
+                CPLDebug("PG",
+                         "Command \"%s\" failed. Trying without 'public'.",
+                         osCommand.c_str());
+                osCommand = CPLSPrintf(
+                    "SET search_path=%s",
+                    OGRPGEscapeString(hPGConn, osActiveSchema.c_str()).c_str());
+                PGresult *hResult2 = OGRPG_PQexec(hPGConn, osCommand.c_str());
+
+                if (!hResult2 || PQresultStatus(hResult2) != PGRES_COMMAND_OK)
+                {
+                    OGRPGClearResult(hResult2);
 
-                CPLError(CE_Failure, CPLE_AppDefined, "%s",
-                         PQerrorMessage(hPGConn));
+                    CPLError(CE_Failure, CPLE_AppDefined, "%s",
+                             PQerrorMessage(hPGConn));
 
-                return FALSE;
+                    return FALSE;
+                }
             }
-        }
 
-        OGRPGClearResult(hResult);
+            OGRPGClearResult(hResult);
+        }
     }
 
     /* -------------------------------------------------------------------- */