Skip to content

Commit f5627e1

Browse files
authored
Merge pull request #9898 from elpaso/backport-9873-to-release-3_4
[postgresql] Fix checkPrimaryKeyUnicity option
2 parents 8131f8c + e75681d commit f5627e1

File tree

9 files changed

+139
-44
lines changed

9 files changed

+139
-44
lines changed

src/app/qgsprojectproperties.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,8 @@ void QgsProjectProperties::apply()
950950
// Set the project title
951951
QgsProject::instance()->setTitle( title() );
952952
QgsProject::instance()->setPresetHomePath( QDir::fromNativeSeparators( mProjectHomeLineEdit->text() ) );
953+
954+
// DB-related options
953955
QgsProject::instance()->setAutoTransaction( mAutoTransaction->isChecked() );
954956
QgsProject::instance()->setEvaluateDefaultValues( mEvaluateDefaultValues->isChecked() );
955957
QgsProject::instance()->setTrustLayerMetadata( mTrustProjectCheckBox->isChecked() );

src/core/qgsvectorlayer.cpp

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,25 +1572,26 @@ QString QgsVectorLayer::loadDefaultStyle( bool &resultFlag )
15721572

15731573
bool QgsVectorLayer::setDataProvider( QString const &provider, const QgsDataProvider::ProviderOptions &options )
15741574
{
1575-
mProviderKey = provider; // XXX is this necessary? Usually already set
1575+
mProviderKey = provider;
1576+
delete mDataProvider;
15761577

1577-
// primary key unicity is tested at construction time, so it has to be set
1578-
// before initializing postgres provider
1579-
QString checkUnicityKey = QStringLiteral( "checkPrimaryKeyUnicity" );
1580-
QString dataSource = mDataSource;
1578+
// For Postgres provider primary key unicity is tested at construction time,
1579+
// so it has to be set before initializing the provider,
1580+
// this manipulation is necessary to preserve default behavior when
1581+
// "trust layer metadata" project level option is set and checkPrimaryKeyUnicity
1582+
// was not explicitly passed in the uri
15811583
if ( provider.compare( QLatin1String( "postgres" ) ) == 0 )
15821584
{
1583-
QgsDataSourceUri uri( dataSource );
1584-
1585-
if ( uri.hasParam( checkUnicityKey ) )
1586-
uri.removeParam( checkUnicityKey );
1587-
1588-
uri.setParam( checkUnicityKey, mReadExtentFromXml ? "0" : "1" );
1589-
dataSource = uri.uri( false );
1585+
const QString checkUnicityKey { QStringLiteral( "checkPrimaryKeyUnicity" ) };
1586+
QgsDataSourceUri uri( mDataSource );
1587+
if ( ! uri.hasParam( checkUnicityKey ) )
1588+
{
1589+
uri.setParam( checkUnicityKey, mReadExtentFromXml ? "0" : "1" );
1590+
mDataSource = uri.uri( false );
1591+
}
15901592
}
15911593

1592-
delete mDataProvider;
1593-
mDataProvider = qobject_cast<QgsVectorDataProvider *>( QgsProviderRegistry::instance()->createProvider( provider, dataSource, options ) );
1594+
mDataProvider = qobject_cast<QgsVectorDataProvider *>( QgsProviderRegistry::instance()->createProvider( provider, mDataSource, options ) );
15941595
if ( !mDataProvider )
15951596
{
15961597
QgsDebugMsgLevel( QStringLiteral( "Unable to get data provider" ), 2 );
@@ -1648,15 +1649,7 @@ bool QgsVectorLayer::setDataProvider( QString const &provider, const QgsDataProv
16481649
if ( !lName.isEmpty() )
16491650
setName( lName );
16501651
}
1651-
16521652
QgsDebugMsgLevel( QStringLiteral( "Beautified layer name %1" ).arg( name() ), 3 );
1653-
1654-
// deal with unnecessary schema qualification to make v.in.ogr happy
1655-
// and remove unnecessary key
1656-
QgsDataSourceUri dataProviderUri( mDataProvider->dataSourceUri() );
1657-
if ( dataProviderUri.hasParam( checkUnicityKey ) )
1658-
dataProviderUri.removeParam( checkUnicityKey );
1659-
mDataSource = dataProviderUri.uri( false );
16601653
}
16611654
else if ( mProviderKey == QLatin1String( "osm" ) )
16621655
{

src/providers/postgres/qgspgtablemodel.cpp

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
#include "qgsdataitem.h"
2020
#include "qgslogger.h"
2121
#include "qgsapplication.h"
22+
#include "qgssettings.h"
23+
#include "qgsproject.h"
2224

2325
#include <climits>
2426

@@ -34,8 +36,11 @@ QgsPgTableModel::QgsPgTableModel()
3436
headerLabels << tr( "SRID" );
3537
headerLabels << tr( "Feature id" );
3638
headerLabels << tr( "Select at id" );
39+
headerLabels << tr( "Check PK unicity" );
3740
headerLabels << tr( "Sql" );
3841
setHorizontalHeaderLabels( headerLabels );
42+
setHeaderData( Columns::DbtmSelectAtId, 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 );
43+
setHeaderData( Columns::DbtmCheckPkUnicity, 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 );
3944
}
4045

4146
void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProperty )
@@ -116,7 +121,25 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
116121
QStandardItem *selItem = new QStandardItem( QString() );
117122
selItem->setFlags( selItem->flags() | Qt::ItemIsUserCheckable );
118123
selItem->setCheckState( Qt::Checked );
119-
selItem->setToolTip( tr( "Disable 'Fast Access to Features at ID' capability to force keeping the attribute table in memory (e.g. in case of expensive views)." ) );
124+
selItem->setToolTip( headerData( Columns::DbtmSelectAtId, Qt::Orientation::Horizontal, Qt::ToolTipRole ).toString() );
125+
126+
QStandardItem *checkPkUnicityItem = new QStandardItem( QString() );
127+
checkPkUnicityItem->setFlags( checkPkUnicityItem->flags() | Qt::ItemIsUserCheckable );
128+
129+
// Legacy: default value is determined by project option to trust layer's metadata
130+
// TODO: remove this default from QGIS 4 and leave default value to false?
131+
// checkPkUnicity has only effect on views and materialized views, so we can safely disable it
132+
if ( layerProperty.isView || layerProperty.isMaterializedView )
133+
{
134+
checkPkUnicityItem->setCheckState( QgsProject::instance( )->trustLayerMetadata() ? Qt::CheckState::Unchecked : Qt::CheckState::Checked );
135+
checkPkUnicityItem->setToolTip( headerData( Columns::DbtmCheckPkUnicity, Qt::Orientation::Horizontal, Qt::ToolTipRole ).toString() );
136+
}
137+
else
138+
{
139+
checkPkUnicityItem->setCheckState( Qt::CheckState::Unchecked );
140+
checkPkUnicityItem->setFlags( checkPkUnicityItem->flags() & ~ Qt::ItemIsEnabled );
141+
checkPkUnicityItem->setToolTip( tr( "This option is only available for views and materialized views." ) );
142+
}
120143

121144
QStandardItem *sqlItem = new QStandardItem( layerProperty.sql );
122145

@@ -131,6 +154,7 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
131154
childItemList << sridItem;
132155
childItemList << pkItem;
133156
childItemList << selItem;
157+
childItemList << checkPkUnicityItem;
134158
childItemList << sqlItem;
135159

136160
Q_FOREACH ( QStandardItem *item, childItemList )
@@ -140,7 +164,7 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
140164
else
141165
item->setFlags( item->flags() & ~Qt::ItemIsSelectable );
142166

143-
if ( tip.isEmpty() )
167+
if ( tip.isEmpty() && item != checkPkUnicityItem && item != selItem )
144168
{
145169
item->setToolTip( QString() );
146170
}
@@ -370,6 +394,7 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
370394

371395
bool selectAtId = itemFromIndex( index.sibling( index.row(), DbtmSelectAtId ) )->checkState() == Qt::Checked;
372396
QString sql = index.sibling( index.row(), DbtmSql ).data( Qt::DisplayRole ).toString();
397+
bool checkPrimaryKeyUnicity = itemFromIndex( index.sibling( index.row(), DbtmCheckPkUnicity ) )->checkState() == Qt::Checked;
373398

374399
QgsDataSourceUri uri( connInfo );
375400

@@ -384,6 +409,7 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
384409
uri.setWkbType( wkbType );
385410
uri.setSrid( srid );
386411
uri.disableSelectAtId( !selectAtId );
412+
uri.setParam( QStringLiteral( "checkPrimaryKeyUnicity" ), checkPrimaryKeyUnicity ? QLatin1Literal( "1" ) : QLatin1Literal( "0" ) );
387413

388414
QgsDebugMsg( QStringLiteral( "returning uri %1" ).arg( uri.uri( false ) ) );
389415
return uri.uri( false );

src/providers/postgres/qgspgtablemodel.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class QgsPgTableModel : public QStandardItemModel
5353
DbtmSrid,
5454
DbtmPkCol,
5555
DbtmSelectAtId,
56+
DbtmCheckPkUnicity,
5657
DbtmSql,
5758
DbtmColumns
5859
};

src/providers/postgres/qgspostgresconn.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ struct QgsPostgresLayerProperty
8383
bool isMaterializedView = false;
8484
QString tableComment;
8585

86-
8786
// TODO: rename this !
8887
int size() const { Q_ASSERT( types.size() == srids.size() ); return types.size(); }
8988

@@ -252,12 +251,16 @@ class QgsPostgresConn : public QObject
252251
PGresult *PQprepare( const QString &stmtName, const QString &query, int nParams, const Oid *paramTypes );
253252
PGresult *PQexecPrepared( const QString &stmtName, const QStringList &params );
254253

255-
//! PQsendQuery is used for asynchronous queries (with PQgetResult)
256-
//! Thread safety must be ensured by the caller by calling QgsPostgresConn::lock() and QgsPostgresConn::unlock()
254+
/**
255+
* PQsendQuery is used for asynchronous queries (with PQgetResult)
256+
* Thread safety must be ensured by the caller by calling QgsPostgresConn::lock() and QgsPostgresConn::unlock()
257+
*/
257258
int PQsendQuery( const QString &query );
258259

259-
//! PQgetResult is used for asynchronous queries (with PQsendQuery)
260-
//! Thread safety must be ensured by the caller by calling QgsPostgresConn::lock() and QgsPostgresConn::unlock()
260+
/**
261+
* PQgetResult is used for asynchronous queries (with PQsendQuery)
262+
* Thread safety must be ensured by the caller by calling QgsPostgresConn::lock() and QgsPostgresConn::unlock()
263+
*/
261264
PGresult *PQgetResult();
262265

263266
bool begin();

src/providers/postgres/qgspostgresprovider.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ QgsPostgresProvider::QgsPostgresProvider( QString const &uri, const ProviderOpti
108108

109109
if ( mUri.hasParam( QStringLiteral( "checkPrimaryKeyUnicity" ) ) )
110110
{
111+
111112
if ( mUri.param( QStringLiteral( "checkPrimaryKeyUnicity" ) ).compare( QLatin1String( "0" ) ) == 0 )
112113
{
113114
mCheckPrimaryKeyUnicity = false;
@@ -1319,7 +1320,7 @@ bool QgsPostgresProvider::determinePrimaryKey()
13191320

13201321
QStringList log;
13211322

1322-
// no primary or unique indizes found
1323+
// no primary or unique indices found
13231324
if ( res.PQntuples() == 0 )
13241325
{
13251326
QgsDebugMsg( QStringLiteral( "Relation has no primary key -- investigating alternatives" ) );
@@ -1575,7 +1576,6 @@ bool QgsPostgresProvider::uniqueData( const QString &quotedColNames )
15751576
pushError( unique.PQresultErrorMessage() );
15761577
return false;
15771578
}
1578-
15791579
return unique.PQntuples() == 1 && unique.PQgetvalue( 0, 0 ).startsWith( 't' );
15801580
}
15811581

src/ui/qgsprojectpropertiesbase.ui

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@
236236
</sizepolicy>
237237
</property>
238238
<property name="currentIndex">
239-
<number>8</number>
239+
<number>4</number>
240240
</property>
241241
<widget class="QWidget" name="mProjOptsGeneral">
242242
<layout class="QVBoxLayout" name="verticalLayout_6">
@@ -265,8 +265,8 @@
265265
<rect>
266266
<x>0</x>
267267
<y>0</y>
268-
<width>590</width>
269-
<height>782</height>
268+
<width>537</width>
269+
<height>795</height>
270270
</rect>
271271
</property>
272272
<layout class="QVBoxLayout" name="verticalLayout_8">
@@ -863,8 +863,8 @@
863863
<rect>
864864
<x>0</x>
865865
<y>0</y>
866-
<width>603</width>
867-
<height>161</height>
866+
<width>546</width>
867+
<height>164</height>
868868
</rect>
869869
</property>
870870
<layout class="QVBoxLayout" name="verticalLayout_7">
@@ -938,8 +938,8 @@
938938
<rect>
939939
<x>0</x>
940940
<y>0</y>
941-
<width>290</width>
942-
<height>552</height>
941+
<width>269</width>
942+
<height>553</height>
943943
</rect>
944944
</property>
945945
<layout class="QVBoxLayout" name="verticalLayout_12">
@@ -1378,7 +1378,7 @@
13781378
<item row="3" column="0">
13791379
<widget class="QCheckBox" name="mTrustProjectCheckBox">
13801380
<property name="toolTip">
1381-
<string>Speed up project loading by skipping data checks. Useful in qgis server context or project with huge database views or materialized views.</string>
1381+
<string>Speed up project loading by skipping data checks in PostgreSQL layers. Useful in QGIS server context or project with huge database views or materialized views.</string>
13821382
</property>
13831383
<property name="text">
13841384
<string>Trust project when data source has no metadata</string>
@@ -1514,8 +1514,8 @@
15141514
<rect>
15151515
<x>0</x>
15161516
<y>0</y>
1517-
<width>168</width>
1518-
<height>54</height>
1517+
<width>167</width>
1518+
<height>55</height>
15191519
</rect>
15201520
</property>
15211521
<layout class="QVBoxLayout" name="verticalLayout_17">
@@ -1575,9 +1575,9 @@
15751575
<property name="geometry">
15761576
<rect>
15771577
<x>0</x>
1578-
<y>-1053</y>
1579-
<width>671</width>
1580-
<height>2645</height>
1578+
<y>0</y>
1579+
<width>598</width>
1580+
<height>2732</height>
15811581
</rect>
15821582
</property>
15831583
<layout class="QVBoxLayout" name="verticalLayout_13">

tests/src/python/test_provider_postgres.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,37 @@ def testReadExtentOnTable(self):
10971097

10981098
self.assertEqual(vl2.extent(), originalExtent)
10991099

1100+
def testCheckPkUnicityOnView(self):
1101+
# vector layer based on view
1102+
1103+
# This is valid
1104+
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')
1105+
self.assertTrue(vl0.isValid())
1106+
1107+
geom = vl0.getFeature(1).geometry().asWkt()
1108+
1109+
# This is NOT valid
1110+
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')
1111+
self.assertFalse(vl0.isValid())
1112+
1113+
# This is NOT valid because the default is to check unicity
1114+
vl0 = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'an_int\' srid=0 type=POINT table="qgis_test"."b21839_pk_unicity_view" (geom) sql=', 'test', 'postgres')
1115+
self.assertFalse(vl0.isValid())
1116+
1117+
# This is valid because the readExtentFromXml option is set
1118+
options = QgsVectorLayer.LayerOptions(True, True) # loadDefaultStyle, readExtentFromXml
1119+
vl0 = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'an_int\' srid=0 type=POINT table="qgis_test"."b21839_pk_unicity_view" (geom) sql=', 'test', 'postgres', options)
1120+
self.assertTrue(vl0.isValid())
1121+
1122+
# Valid because a_unique_int is unique and default is to check unicity
1123+
vl0 = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'a_unique_int\' srid=0 type=POINT table="qgis_test"."b21839_pk_unicity_view" (geom) sql=', 'test', 'postgres')
1124+
self.assertEqual(vl0.getFeature(1).geometry().asWkt(), geom)
1125+
1126+
# Valid because a_unique_int is unique
1127+
vl0 = QgsVectorLayer(self.dbconn + ' checkPrimaryKeyUnicity=\'1\' sslmode=disable key=\'a_unique_int\' srid=0 type=POINT table="qgis_test"."b21839_pk_unicity_view" (geom) sql=', 'test', 'postgres')
1128+
self.assertTrue(vl0.isValid())
1129+
self.assertEqual(vl0.getFeature(1).geometry().asWkt(), geom)
1130+
11001131
def testNotify(self):
11011132
vl0 = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'pk\' srid=4326 type=POLYGON table="qgis_test"."some_poly_data" (geom) sql=', 'test', 'postgres')
11021133
vl0.dataProvider().setListening(True)

tests/testdata/provider/testdata_pg.sql

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,4 +534,43 @@ INSERT INTO qgis_test.check_constraints VALUES (
534534
1, -- id
535535
4, -- a
536536
3 -- b
537+
);
538+
539+
540+
---------------------------------------------
541+
--
542+
-- Table and view for tests on checkPrimaryKeyUnicity
543+
--
544+
545+
DROP TABLE IF EXISTS qgis_test.b21839_pk_unicity CASCADE;
546+
CREATE TABLE qgis_test.b21839_pk_unicity
547+
(
548+
pk serial NOT NULL,
549+
an_int integer NOT NULL,
550+
a_unique_int integer NOT NULL,
551+
geom geometry(Point),
552+
CONSTRAINT b21839_pk_unicity_pkey PRIMARY KEY (pk)
537553
)
554+
WITH (
555+
OIDS=FALSE
556+
);
557+
558+
559+
INSERT INTO qgis_test.b21839_pk_unicity(
560+
pk, an_int, a_unique_int , geom)
561+
VALUES (1, 1, 1, ST_GeomFromText('point( 1 1)'));
562+
563+
564+
INSERT INTO qgis_test.b21839_pk_unicity(
565+
pk, an_int, a_unique_int, geom)
566+
VALUES (2, 1, 2, ST_GeomFromText('point( 1 3)'));
567+
568+
569+
570+
CREATE OR REPLACE VIEW qgis_test.b21839_pk_unicity_view AS
571+
SELECT b21839_pk_unicity.pk,
572+
b21839_pk_unicity.an_int,
573+
b21839_pk_unicity.a_unique_int,
574+
b21839_pk_unicity.geom
575+
FROM qgis_test.b21839_pk_unicity;
576+

0 commit comments

Comments
 (0)