[postgres] Fix checkPrimaryKeyUnicity option

This provider option was linked to the project level option
"Trust layer metadata..." which was implemented
to speed up loading of large dataset by trusting extent
read from metadata to avoid costly operations to determine
the layer extent.

Check PK unicity on the other hand has only effect on views
and query layers and it is useful as an independent
option to prevent loading of layers that have no PK (or the
wrong one).

But the operation of determine unicity of a values in a column
can also be costly, so better to get control back to the user.

Legacy default is preserved (the project-level "Trust..." option).

Fixes #21839

Funded by RAAB.nl

Author: elpaso

src/app/qgsprojectproperties.cpp

@@ -989,6 +989,8 @@ void QgsProjectProperties::apply()
   // Set the project title
   QgsProject::instance()->setTitle( title() );
   QgsProject::instance()->setPresetHomePath( QDir::fromNativeSeparators( mProjectHomeLineEdit->text() ) );
+
+  // DB-related options
   QgsProject::instance()->setAutoTransaction( mAutoTransaction->isChecked() );
   QgsProject::instance()->setEvaluateDefaultValues( mEvaluateDefaultValues->isChecked() );
   QgsProject::instance()->setTrustLayerMetadata( mTrustProjectCheckBox->isChecked() );

src/core/qgsvectorlayer.cpp

@@ -1560,24 +1560,8 @@ QString QgsVectorLayer::loadDefaultStyle( bool &resultFlag )
 bool QgsVectorLayer::setDataProvider( QString const &provider, const QgsDataProvider::ProviderOptions &options )
 {
   mProviderKey = provider;
-
-  // primary key unicity is tested at construction time, so it has to be set
-  // before initializing postgres provider
-  QString checkUnicityKey = QStringLiteral( "checkPrimaryKeyUnicity" );
-  QString dataSource = mDataSource;
-  if ( provider.compare( QLatin1String( "postgres" ) ) == 0 )
-  {
-    QgsDataSourceUri uri( dataSource );
-
-    if ( uri.hasParam( checkUnicityKey ) )
-      uri.removeParam( checkUnicityKey );
-
-    uri.setParam( checkUnicityKey, mReadExtentFromXml ? "0" : "1" );
-    dataSource = uri.uri( false );
-  }
-
   delete mDataProvider;
-  mDataProvider = qobject_cast<QgsVectorDataProvider *>( QgsProviderRegistry::instance()->createProvider( provider, dataSource, options ) );
+  mDataProvider = qobject_cast<QgsVectorDataProvider *>( QgsProviderRegistry::instance()->createProvider( provider, mDataSource, options ) );
   if ( !mDataProvider )
   {
     mValid = false;
@@ -1635,15 +1619,7 @@ bool QgsVectorLayer::setDataProvider( QString const &provider, const QgsDataProv
       if ( !lName.isEmpty() )
         setName( lName );
     }
-
     QgsDebugMsgLevel( QStringLiteral( "Beautified layer name %1" ).arg( name() ), 3 );
-
-    // deal with unnecessary schema qualification to make v.in.ogr happy
-    // and remove unnecessary key
-    QgsDataSourceUri dataProviderUri( mDataProvider->dataSourceUri() );
-    if ( dataProviderUri.hasParam( checkUnicityKey ) )
-      dataProviderUri.removeParam( checkUnicityKey );
-    mDataSource = dataProviderUri.uri( false );
   }
   else if ( mProviderKey == QLatin1String( "osm" ) )
   {

src/providers/postgres/qgspgtablemodel.cpp

@@ -20,6 +20,7 @@
 #include "qgslogger.h"
 #include "qgsapplication.h"
 #include "qgssettings.h"
+#include "qgsproject.h"
 
 #include <climits>
 
@@ -39,7 +40,7 @@ QgsPgTableModel::QgsPgTableModel()
   headerLabels << tr( "Sql" );
   setHorizontalHeaderLabels( headerLabels );
   setHeaderData( 8, Qt::Orientation::Horizontal, tr( "Disable 'Fast Access to Features at ID' capability to force keeping the attribute table in memory (e.g. in case of expensive views)." ), Qt::ToolTipRole );
-  setHeaderData( 9, Qt::Orientation::Horizontal, tr( "Enable check for primary key unicity when loading the features. This may slow down loading for large tables." ), Qt::ToolTipRole );
+  setHeaderData( 9, Qt::Orientation::Horizontal, tr( "Enable check for primary key unicity when loading views and materialized views. This option can make loading of large datasets significantly slower." ), Qt::ToolTipRole );
 }
 
 void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProperty )
@@ -127,8 +128,21 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
 
     QStandardItem *checkPkUnicityItem  = new QStandardItem( QString() );
     checkPkUnicityItem->setFlags( checkPkUnicityItem->flags() | Qt::ItemIsUserCheckable );
-    checkPkUnicityItem->setCheckState( Qt::Unchecked );
-    checkPkUnicityItem->setToolTip( headerData( 9, Qt::Orientation::Horizontal, Qt::ToolTipRole ).toString() );
+
+    // Legacy: default value is determined by project option to trust layer's metadata
+    // TODO: remove this default from QGIS 4 and leave default value to false
+    // checkPkUnicity has only effect on views and materialized views, so we can safely disable it
+    if ( layerProperty.isView || layerProperty.isMaterializedView )
+    {
+      checkPkUnicityItem->setCheckState( QgsProject::instance( )->trustLayerMetadata() ? Qt::CheckState::Unchecked : Qt::CheckState::Checked );
+      checkPkUnicityItem->setToolTip( headerData( 9, Qt::Orientation::Horizontal, Qt::ToolTipRole ).toString() );
+    }
+    else
+    {
+      checkPkUnicityItem->setCheckState( Qt::CheckState::Unchecked );
+      checkPkUnicityItem->setFlags( checkPkUnicityItem->flags() & ~ Qt::ItemIsEnabled );
+      checkPkUnicityItem->setToolTip( tr( "This option is only available for views and materialized views." ) );
+    }
 
     QStandardItem *sqlItem = new QStandardItem( layerProperty.sql );
 
@@ -383,7 +397,7 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
 
   bool selectAtId = itemFromIndex( index.sibling( index.row(), DbtmSelectAtId ) )->checkState() == Qt::Checked;
   QString sql = index.sibling( index.row(), DbtmSql ).data( Qt::DisplayRole ).toString();
-  bool checkPkUnicity = itemFromIndex( index.sibling( index.row(), DbtmCheckPkUnicity ) )->checkState() == Qt::Checked;
+  bool checkPrimaryKeyUnicity = itemFromIndex( index.sibling( index.row(), DbtmCheckPkUnicity ) )->checkState() == Qt::Checked;
 
   QgsDataSourceUri uri( connInfo );
 
@@ -400,7 +414,7 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
   uri.setWkbType( wkbType );
   uri.setSrid( srid );
   uri.disableSelectAtId( !selectAtId );
-  uri.setParam( QStringLiteral( "checkPrimaryKeyUnicity" ), QString( checkPkUnicity ) );
+  uri.setParam( QStringLiteral( "checkPrimaryKeyUnicity" ), checkPrimaryKeyUnicity ? QLatin1Literal( "1" ) : QLatin1Literal( "0" ) );
 
   QgsDebugMsg( QStringLiteral( "returning uri %1" ).arg( uri.uri( false ) ) );
   return uri.uri( false );

src/providers/postgres/qgspostgresconn.h

@@ -82,8 +82,6 @@ struct QgsPostgresLayerProperty
   bool                          isView = false;
   bool                          isMaterializedView = false;
   QString                       tableComment;
-  bool                          checkPkUnicity = false;
-
 
   // TODO: rename this !
   int size() const { Q_ASSERT( types.size() == srids.size() ); return types.size(); }

src/providers/postgres/qgspostgresprovider.cpp

@@ -1312,7 +1312,7 @@ bool QgsPostgresProvider::determinePrimaryKey()
 
     QStringList log;
 
-    // no primary or unique indizes found
+    // no primary or unique indices found
     if ( res.PQntuples() == 0 )
     {
       QgsDebugMsg( QStringLiteral( "Relation has no primary key -- investigating alternatives" ) );
@@ -1568,7 +1568,6 @@ bool QgsPostgresProvider::uniqueData( const QString &quotedColNames )
     pushError( unique.PQresultErrorMessage() );
     return false;
   }
-
   return unique.PQntuples() == 1 && unique.PQgetvalue( 0, 0 ).startsWith( 't' );
 }
 

src/ui/qgsprojectpropertiesbase.ui

@@ -236,7 +236,7 @@
           </sizepolicy>
          </property>
          <property name="currentIndex">
-          <number>8</number>
+          <number>4</number>
          </property>
          <widget class="QWidget" name="mProjOptsGeneral">
           <layout class="QVBoxLayout" name="verticalLayout_6">
@@ -265,8 +265,8 @@
                <rect>
                 <x>0</x>
                 <y>0</y>
-                <width>590</width>
-                <height>777</height>
+                <width>671</width>
+                <height>795</height>
                </rect>
               </property>
               <layout class="QVBoxLayout" name="verticalLayout_8">
@@ -863,8 +863,8 @@
                <rect>
                 <x>0</x>
                 <y>0</y>
-                <width>603</width>
-                <height>161</height>
+                <width>685</width>
+                <height>780</height>
                </rect>
               </property>
               <layout class="QVBoxLayout" name="verticalLayout_7">
@@ -938,8 +938,8 @@
                <rect>
                 <x>0</x>
                 <y>0</y>
-                <width>290</width>
-                <height>543</height>
+                <width>685</width>
+                <height>780</height>
                </rect>
               </property>
               <layout class="QVBoxLayout" name="verticalLayout_12">
@@ -1378,7 +1378,7 @@
            <item row="3" column="0">
             <widget class="QCheckBox" name="mTrustProjectCheckBox">
              <property name="toolTip">
-              <string>Speed up project loading by skipping data checks. Useful in qgis server context or project with huge database views or materialized views.</string>
+              <string>Speed up project loading by skipping data checks in Postgres layers. Useful in QGIS server context or project with huge database views or materialized views.</string>
              </property>
              <property name="text">
               <string>Trust project when data source has no metadata</string>
@@ -1514,8 +1514,8 @@
                <rect>
                 <x>0</x>
                 <y>0</y>
-                <width>178</width>
-                <height>54</height>
+                <width>167</width>
+                <height>55</height>
                </rect>
               </property>
               <layout class="QVBoxLayout" name="verticalLayout_17">
@@ -1575,9 +1575,9 @@
               <property name="geometry">
                <rect>
                 <x>0</x>
-                <y>-975</y>
+                <y>0</y>
                 <width>671</width>
-                <height>2682</height>
+                <height>2732</height>
                </rect>
               </property>
               <layout class="QVBoxLayout" name="verticalLayout_13">

tests/src/python/test_provider_postgres.py

@@ -1097,6 +1097,21 @@ def testReadExtentOnTable(self):
 
         self.assertEqual(vl2.extent(), originalExtent)
 
+    def testCheckPkUnicityOnView(self):
+        # vector layer based on view
+
+        # This is valid
+        vl0 = QgsVectorLayer(self.dbconn + ' checkPrimaryKeyUnicity=\'0\'  sslmode=disable key=\'pk\' srid=0 type=POINT table="qgis_test"."b21839_pk_unicity_view" (geom) sql=', 'test', 'postgres')
+        self.assertTrue(vl0.isValid())
+
+        # This is NOT valid
+        vl0 = QgsVectorLayer(self.dbconn + ' checkPrimaryKeyUnicity=\'1\' sslmode=disable key=\'an_int\' srid=0 type=POINT table="qgis_test"."b21839_pk_unicity_view" (geom) sql=', 'test', 'postgres')
+        self.assertFalse(vl0.isValid())
+
+        # This is valid because the default is to not check unicity
+        vl0 = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'an_int\' srid=0 type=POINT table="qgis_test"."b21839_pk_unicity_view" (geom) sql=', 'test', 'postgres')
+        self.assertFalse(vl0.isValid())
+
     def testNotify(self):
         vl0 = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'pk\' srid=4326 type=POLYGON table="qgis_test"."some_poly_data" (geom) sql=', 'test', 'postgres')
         vl0.dataProvider().setListening(True)

tests/testdata/provider/testdata_pg.sql

@@ -535,3 +535,41 @@ INSERT INTO qgis_test.check_constraints VALUES (
   4, -- a
   3  -- b
 )
+
+
+---------------------------------------------
+--
+-- Table and view for tests on  checkPrimaryKeyUnicity
+--
+
+
+DROP TABLE IF EXISTS qgis_test.b21839_pk_unicity CASCADE;
+CREATE TABLE qgis_test.b21839_pk_unicity
+(
+  pk serial NOT NULL,
+  an_int integer NOT NULL,
+  geom geometry(Point),
+  CONSTRAINT b21839_pk_unicity_pkey PRIMARY KEY (pk)
+)
+WITH (
+  OIDS=FALSE
+);
+
+
+INSERT INTO qgis_test.b21839_pk_unicity(
+            pk, an_int, geom)
+    VALUES (1, 1, ST_GeomFromText('point( 1 1)'));
+
+
+INSERT INTO qgis_test.b21839_pk_unicity(
+            pk, an_int, geom)
+    VALUES (2, 1, ST_GeomFromText('point( 1 3)'));
+
+
+
+CREATE OR REPLACE VIEW qgis_test.b21839_pk_unicity_view AS
+ SELECT b21839_pk_unicity.pk,
+    b21839_pk_unicity.an_int,
+    b21839_pk_unicity.geom
+   FROM qgis_test.b21839_pk_unicity;
+