[FEATURE] postgres provider: save primary key selection

When a view loaded from Data Source Managers's PostgreSQL tab you can
select the key columns of the view (by default the first column is used,
which can be wrong). This commit stores that selection in the settings,
so that it doesn't have to be reselected on subsequent loads.  This
stored selection is also used when adding the loading from the browser.

src/providers/postgres/qgspgsourceselect.cpp

@@ -539,6 +539,7 @@ void QgsPgSourceSelect::btnConnect_clicked()
 
   QModelIndex rootItemIndex = mTableModel.indexFromItem( mTableModel.invisibleRootItem() );
   mTableModel.removeRows( 0, mTableModel.rowCount( rootItemIndex ), rootItemIndex );
+  mTableModel.setConnectionName( cmbConnections->currentText() );
 
   // populate the table list
   QgsDataSourceUri uri = QgsPostgresConn::connUri( cmbConnections->currentText() );

src/providers/postgres/qgspgtablemodel.cpp

@@ -19,6 +19,7 @@
 #include "qgsdataitem.h"
 #include "qgslogger.h"
 #include "qgsapplication.h"
+#include "qgssettings.h"
 
 #include <climits>
 
@@ -101,18 +102,21 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
       pkItem->setFlags( pkItem->flags() & ~Qt::ItemIsEditable );
 
     pkItem->setData( layerProperty.pkCols, Qt::UserRole + 1 );
-    pkItem->setData( "", Qt::UserRole + 2 );
 
-    if ( !layerProperty.pkCols.isEmpty() )
+    QStringList defPk( QgsSettings().value( QStringLiteral( "/PostgreSQL/connections/%1/keys/%2/%3" ).arg( mConnName, layerProperty.schemaName, layerProperty.tableName ), QStringList() ).toStringList() );
+
+    if ( !layerProperty.pkCols.isEmpty() && defPk.isEmpty() )
     {
       // If we have a view with multiple possible columns to be used as the primary key, for convenience
       // let's select the first one - this is what the browser dock already does. We risk that a wrong column
       // will be used, but most of the time we should be fine.
-      QString firstCol = layerProperty.pkCols[0];
-      pkItem->setText( firstCol );
-      pkItem->setData( QStringList( firstCol ), Qt::UserRole + 2 );
+      defPk = QStringList( layerProperty.pkCols[0] );
     }
 
+    pkItem->setData( defPk, Qt::UserRole + 2 );
+    if ( !defPk.isEmpty() )
+      pkItem->setText( defPk.join( ',' ) );
+
     QStandardItem *selItem = new QStandardItem( QString() );
     selItem->setFlags( selItem->flags() | Qt::ItemIsUserCheckable );
     selItem->setCheckState( Qt::Checked );
@@ -379,7 +383,9 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
     cols << QgsPostgresConn::quotedIdentifier( col );
   }
 
-  uri.setDataSource( schemaName, tableName, geomColumnName, sql, cols.join( QStringLiteral( "," ) ) );
+  QgsSettings().setValue( QStringLiteral( "/PostgreSQL/connections/%1/keys/%2/%3" ).arg( mConnName, schemaName, tableName ), QVariant( s1.toList() ) );
+
+  uri.setDataSource( schemaName, tableName, geomColumnName, sql, cols.join( ',' ) );
   uri.setUseEstimatedMetadata( useEstimatedMetadata );
   uri.setWkbType( wkbType );
   uri.setSrid( srid );

src/providers/postgres/qgspgtablemodel.h

@@ -63,9 +63,13 @@ class QgsPgTableModel : public QStandardItemModel
 
     static QIcon iconForWkbType( QgsWkbTypes::Type type );
 
+    void setConnectionName( const QString &connName ) { mConnName = connName;  }
+
   private:
     //! Number of tables in the model
     int mTableCount = 0;
+    //! connection name
+    QString mConnName;
 };
 
 #endif // QGSPGTABLEMODEL_H

src/providers/postgres/qgspostgresconn.cpp

@@ -1878,6 +1878,7 @@ void QgsPostgresConn::deleteConnection( const QString &connName )
   settings.remove( key + "/savePassword" );
   settings.remove( key + "/save" );
   settings.remove( key + "/authcfg" );
+  settings.remove( key + "/keys" );
   settings.remove( key );
 }
 

src/providers/postgres/qgspostgresdataitems.cpp

@@ -24,6 +24,7 @@
 #include "qgsmessageoutput.h"
 #include "qgsprojectstorageregistry.h"
 #include "qgsvectorlayer.h"
+#include "qgssettings.h"
 
 #ifdef HAVE_GUI
 #include "qgspgnewconnection.h"
@@ -54,7 +55,7 @@ QVector<QgsDataItem *> QgsPGConnectionItem::createChildren()
   QVector<QgsDataItem *>items;
 
   QgsDataSourceUri uri = QgsPostgresConn::connUri( mName );
-  // TODO: wee need to cancel somehow acquireConnection() if deleteLater() was called on this item to avoid later credential dialog if connection failed
+  // TODO: we need to cancel somehow acquireConnection() if deleteLater() was called on this item to avoid later credential dialog if connection failed
   QgsPostgresConn *conn = QgsPostgresConnPool::instance()->acquireConnection( uri.connectionInfo( false ) );
   if ( !conn )
   {
@@ -492,7 +493,6 @@ void QgsPGLayerItem::refreshMaterializedView()
 
 QString QgsPGLayerItem::createUri()
 {
-  QString pkColName = !mLayerProperty.pkCols.isEmpty() ? mLayerProperty.pkCols.at( 0 ) : QString();
   QgsPGConnectionItem *connItem = qobject_cast<QgsPGConnectionItem *>( parent() ? parent()->parent() : nullptr );
 
   if ( !connItem )
@@ -502,7 +502,19 @@ QString QgsPGLayerItem::createUri()
   }
 
   QgsDataSourceUri uri( QgsPostgresConn::connUri( connItem->name() ).connectionInfo( false ) );
-  uri.setDataSource( mLayerProperty.schemaName, mLayerProperty.tableName, mLayerProperty.geometryColName, mLayerProperty.sql, pkColName );
+
+  QStringList defPk( QgsSettings().value(
+                       QStringLiteral( "/PostgreSQL/connections/%1/keys/%2/%3" ).arg( connItem->name(), mLayerProperty.schemaName, mLayerProperty.tableName ),
+                       QVariant( !mLayerProperty.pkCols.isEmpty() ? QStringList( mLayerProperty.pkCols.at( 0 ) ) : QStringList() )
+                     ).toStringList() );
+
+  QStringList cols;
+  for ( const auto &col : defPk )
+  {
+    cols << QgsPostgresConn::quotedIdentifier( col );
+  }
+
+  uri.setDataSource( mLayerProperty.schemaName, mLayerProperty.tableName, mLayerProperty.geometryColName, mLayerProperty.sql, cols.join( ',' ) );
   uri.setWkbType( mLayerProperty.types.at( 0 ) );
   if ( uri.wkbType() != QgsWkbTypes::NoGeometry )
     uri.setSrid( QString::number( mLayerProperty.srids.at( 0 ) ) );