Merge pull request #38981 from elpaso/bugfix-gh38975-and-other-db-man…

…ager-issues Bugfix gh38975 and other db manager issues
qgis · Sep 24, 2020 · 3b6178a · 3b6178a
2 parents 6bd03c3 + dd64c6d
commit 3b6178a
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 25 deletions.
diff --git a/python/plugins/db_manager/db_plugins/postgis/connector.py b/python/plugins/db_manager/db_plugins/postgis/connector.py
@@ -29,7 +29,6 @@
 from qgis.PyQt.QtCore import (
     QRegExp,
     QFile,
-    QCoreApplication,
     QVariant,
     QDateTime,
     QTime,
@@ -39,7 +38,6 @@
 from qgis.core import (
     Qgis,
     QgsCoordinateReferenceSystem,
-    QgsCredentials,
     QgsVectorLayer,
     QgsDataSourceUri,
     QgsProviderRegistry,
@@ -48,9 +46,10 @@
 )
 
 from ..connector import DBConnector
-from ..plugin import ConnectionError, DbError, Table
+from ..plugin import DbError, Table
 
 import os
+import re
 import psycopg2
 import psycopg2.extensions
 
@@ -118,29 +117,49 @@ def description(self):
 
         if self._description is None:
 
-            uri = QgsDataSourceUri(self.connection.uri())
+            self._description = []
 
-            # TODO: make this part provider-agnostic
-            sql = self.sql if self.sql.upper().find(' LIMIT ') >= 0 else self.sql + ' LIMIT 1 '
-            uri.setTable('(SELECT row_number() OVER () AS __rid__, * FROM (' + sql + ') as foo)')
-            uri.setKeyColumn('__rid__')
-            # TODO: fetch provider name from connection (QgsAbstractConnectionProvider)
-            # TODO: re-use the VectorLayer for fetching rows in batch mode
-            vl = QgsVectorLayer(uri.uri(False), 'dbmanager_cursor', 'postgres')
+            if re.match('^SHOW', self.sql.strip().upper()):
+                try:
+                    count = len(self.connection.executeSql(self.sql)[0])
+                except QgsProviderConnectionException:
+                    count = 1
+                for i in range(count):
+                    self._description.append([
+                        '',  # name
+                        '',  # type_code
+                        -1,  # display_size
+                        -1,  # internal_size
+                        -1,  # precision
+                        None,  # scale
+                        True  # null_ok
+                    ])
+            else:
+                uri = QgsDataSourceUri(self.connection.uri())
+
+                # TODO: make this part provider-agnostic
+                sql = self.sql if self.sql.upper().find(' LIMIT ') >= 0 else self.sql + ' LIMIT 1 '
+                uri.setTable('(SELECT row_number() OVER () AS __rid__, * FROM (' + sql + ') as foo)')
+                uri.setKeyColumn('__rid__')
+                uri.setParam('checkPrimaryKeyUnicity', '0')
+                # TODO: fetch provider name from connection (QgsAbstractConnectionProvider)
+                # TODO: re-use the VectorLayer for fetching rows in batch mode
+                vl = QgsVectorLayer(uri.uri(False), 'dbmanager_cursor', 'postgres')
+
+                fields = vl.fields()
+
+                for i in range(1, len(fields)):  # skip first field (__rid__)
+                    f = fields[i]
+                    self._description.append([
+                        f.name(),  # name
+                        f.type(),  # type_code
+                        f.length(),  # display_size
+                        f.length(),  # internal_size
+                        f.precision(),  # precision
+                        None,  # scale
+                        True  # null_ok
+                    ])
 
-            fields = vl.fields()
-            self._description = []
-            for i in range(1, len(fields)):  # skip first field (__rid__)
-                f = fields[i]
-                self._description.append([
-                    f.name(),  # name
-                    f.type(),  # type_code
-                    f.length(),  # display_size
-                    f.length(),  # internal_size
-                    f.precision(),  # precision
-                    None,  # scale
-                    True  # null_ok
-                ])
             self._debug("get_description returned " + str(len(self._description)) + " cols")
 
         return self._description

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -1175,11 +1175,24 @@ bool QgsPostgresProvider::loadFields()
     if ( fields.contains( fieldName ) )
     {
       QgsMessageLog::logMessage( tr( "Duplicate field %1 found\n" ).arg( fieldName ), tr( "PostGIS" ) );
-      // In case of read-only query layers we can safely ignore the issue
+      // In case of read-only query layers we can safely ignore the issue and rename the duplicated field
       if ( ! mIsQuery )
       {
         return false;
       }
+      else
+      {
+        unsigned short int i = 1;
+        while ( i < std::numeric_limits<unsigned short int>::max() )
+        {
+          const QString newName { QStringLiteral( "%1 (%2)" ).arg( fieldName ).arg( ++i ) };
+          if ( ! fields.contains( newName ) )
+          {
+            fieldName = newName;
+            break;
+          }
+        }
+      }
     }
 
     fields << fieldName;

diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py
@@ -3021,6 +3021,23 @@ def testTrustFlag(self):
         vl = p.mapLayersByName('testTrustFlag')[0]
         self.assertTrue(vl.isValid())
 
+    def testQueryLayerDuplicatedFields(self):
+        """Test that duplicated fields from a query layer are returned"""
+
+        def _get_layer(sql):
+            return QgsVectorLayer(
+                self.dbconn +
+                ' sslmode=disable key=\'__rid__\' table=\'(SELECT row_number() OVER () AS __rid__, * FROM (' + sql + ') as foo)\'  sql=',
+                'test', 'postgres')
+
+        l = _get_layer('SELECT 1, 2')
+        self.assertEqual(l.fields().count(), 3)
+        self.assertEqual([f.name() for f in l.fields()], ['__rid__', '?column?', '?column? (2)'])
+
+        l = _get_layer('SELECT 1 as id, 2 as id')
+        self.assertEqual(l.fields().count(), 3)
+        self.assertEqual([f.name() for f in l.fields()], ['__rid__', 'id', 'id (2)'])
+
 
 if __name__ == '__main__':
     unittest.main()