Skip to content

Commit 4d2a8dd

Browse files
elpasoelpaso
authoredApr 27, 2019
Merge pull request #9873 from elpaso/bugfix-21839-postgresql-check-pk-unicity
[postgresql] Fix checkPrimaryKeyUnicity option
·
ltr-3_40final-3_8_0
2 parents ffaa64e + 3a1f6d7 commit 4d2a8dd

File tree

9 files changed

+130
-40
lines changed

9 files changed

+130
-40
lines changed
 

‎src/app/qgsprojectproperties.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -995,6 +995,8 @@ void QgsProjectProperties::apply()
995995
// Set the project title
996996
QgsProject::instance()->setTitle( title() );
997997
QgsProject::instance()->setPresetHomePath( QDir::fromNativeSeparators( mProjectHomeLineEdit->text() ) );
998+
999+
// DB-related options
9981000
QgsProject::instance()->setAutoTransaction( mAutoTransaction->isChecked() );
9991001
QgsProject::instance()->setEvaluateDefaultValues( mEvaluateDefaultValues->isChecked() );
10001002
QgsProject::instance()->setTrustLayerMetadata( mTrustProjectCheckBox->isChecked() );

‎src/core/qgsvectorlayer.cpp

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

15821582
bool QgsVectorLayer::setDataProvider( QString const &provider, const QgsDataProvider::ProviderOptions &options )
15831583
{
1584-
mProviderKey = provider; // XXX is this necessary? Usually already set
1584+
mProviderKey = provider;
1585+
delete mDataProvider;
15851586

1586-
// primary key unicity is tested at construction time, so it has to be set
1587-
// before initializing postgres provider
1588-
QString checkUnicityKey = QStringLiteral( "checkPrimaryKeyUnicity" );
1589-
QString dataSource = mDataSource;
1587+
// For Postgres provider primary key unicity is tested at construction time,
1588+
// so it has to be set before initializing the provider,
1589+
// this manipulation is necessary to preserve default behavior when
1590+
// "trust layer metadata" project level option is set and checkPrimaryKeyUnicity
1591+
// was not explicitly passed in the uri
15901592
if ( provider.compare( QLatin1String( "postgres" ) ) == 0 )
15911593
{
1592-
QgsDataSourceUri uri( dataSource );
1593-
1594-
if ( uri.hasParam( checkUnicityKey ) )
1595-
uri.removeParam( checkUnicityKey );
1596-
1597-
uri.setParam( checkUnicityKey, mReadExtentFromXml ? "0" : "1" );
1598-
dataSource = uri.uri( false );
1594+
const QString checkUnicityKey { QStringLiteral( "checkPrimaryKeyUnicity" ) };
1595+
QgsDataSourceUri uri( mDataSource );
1596+
if ( ! uri.hasParam( checkUnicityKey ) )
1597+
{
1598+
uri.setParam( checkUnicityKey, mReadExtentFromXml ? "0" : "1" );
1599+
mDataSource = uri.uri( false );
1600+
}
15991601
}
16001602

1601-
delete mDataProvider;
1602-
mDataProvider = qobject_cast<QgsVectorDataProvider *>( QgsProviderRegistry::instance()->createProvider( provider, dataSource, options ) );
1603+
mDataProvider = qobject_cast<QgsVectorDataProvider *>( QgsProviderRegistry::instance()->createProvider( provider, mDataSource, options ) );
16031604
if ( !mDataProvider )
16041605
{
16051606
mValid = false;
@@ -1658,15 +1659,7 @@ bool QgsVectorLayer::setDataProvider( QString const &provider, const QgsDataProv
16581659
if ( !lName.isEmpty() )
16591660
setName( lName );
16601661
}
1661-
16621662
QgsDebugMsgLevel( QStringLiteral( "Beautified layer name %1" ).arg( name() ), 3 );
1663-
1664-
// deal with unnecessary schema qualification to make v.in.ogr happy
1665-
// and remove unnecessary key
1666-
QgsDataSourceUri dataProviderUri( mDataProvider->dataSourceUri() );
1667-
if ( dataProviderUri.hasParam( checkUnicityKey ) )
1668-
dataProviderUri.removeParam( checkUnicityKey );
1669-
mDataSource = dataProviderUri.uri( false );
16701663
}
16711664
else if ( mProviderKey == QLatin1String( "osm" ) )
16721665
{

‎src/providers/postgres/qgspgtablemodel.cpp

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "qgslogger.h"
2121
#include "qgsapplication.h"
2222
#include "qgssettings.h"
23+
#include "qgsproject.h"
2324

2425
#include <climits>
2526

@@ -35,8 +36,11 @@ QgsPgTableModel::QgsPgTableModel()
3536
headerLabels << tr( "SRID" );
3637
headerLabels << tr( "Feature id" );
3738
headerLabels << tr( "Select at id" );
39+
headerLabels << tr( "Check PK unicity" );
3840
headerLabels << tr( "Sql" );
3941
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 );
4044
}
4145

4246
void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProperty )
@@ -120,7 +124,25 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
120124
QStandardItem *selItem = new QStandardItem( QString() );
121125
selItem->setFlags( selItem->flags() | Qt::ItemIsUserCheckable );
122126
selItem->setCheckState( Qt::Checked );
123-
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)." ) );
127+
selItem->setToolTip( headerData( Columns::DbtmSelectAtId, Qt::Orientation::Horizontal, Qt::ToolTipRole ).toString() );
128+
129+
QStandardItem *checkPkUnicityItem = new QStandardItem( QString() );
130+
checkPkUnicityItem->setFlags( checkPkUnicityItem->flags() | Qt::ItemIsUserCheckable );
131+
132+
// Legacy: default value is determined by project option to trust layer's metadata
133+
// TODO: remove this default from QGIS 4 and leave default value to false?
134+
// checkPkUnicity has only effect on views and materialized views, so we can safely disable it
135+
if ( layerProperty.isView || layerProperty.isMaterializedView )
136+
{
137+
checkPkUnicityItem->setCheckState( QgsProject::instance( )->trustLayerMetadata() ? Qt::CheckState::Unchecked : Qt::CheckState::Checked );
138+
checkPkUnicityItem->setToolTip( headerData( Columns::DbtmCheckPkUnicity, Qt::Orientation::Horizontal, Qt::ToolTipRole ).toString() );
139+
}
140+
else
141+
{
142+
checkPkUnicityItem->setCheckState( Qt::CheckState::Unchecked );
143+
checkPkUnicityItem->setFlags( checkPkUnicityItem->flags() & ~ Qt::ItemIsEnabled );
144+
checkPkUnicityItem->setToolTip( tr( "This option is only available for views and materialized views." ) );
145+
}
124146

125147
QStandardItem *sqlItem = new QStandardItem( layerProperty.sql );
126148

@@ -135,6 +157,7 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
135157
childItemList << sridItem;
136158
childItemList << pkItem;
137159
childItemList << selItem;
160+
childItemList << checkPkUnicityItem;
138161
childItemList << sqlItem;
139162

140163
const auto constChildItemList = childItemList;
@@ -145,7 +168,7 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
145168
else
146169
item->setFlags( item->flags() & ~Qt::ItemIsSelectable );
147170

148-
if ( tip.isEmpty() )
171+
if ( tip.isEmpty() && item != checkPkUnicityItem && item != selItem )
149172
{
150173
item->setToolTip( QString() );
151174
}
@@ -375,6 +398,7 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
375398

376399
bool selectAtId = itemFromIndex( index.sibling( index.row(), DbtmSelectAtId ) )->checkState() == Qt::Checked;
377400
QString sql = index.sibling( index.row(), DbtmSql ).data( Qt::DisplayRole ).toString();
401+
bool checkPrimaryKeyUnicity = itemFromIndex( index.sibling( index.row(), DbtmCheckPkUnicity ) )->checkState() == Qt::Checked;
378402

379403
QgsDataSourceUri uri( connInfo );
380404

@@ -392,6 +416,7 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
392416
uri.setWkbType( wkbType );
393417
uri.setSrid( srid );
394418
uri.disableSelectAtId( !selectAtId );
419+
uri.setParam( QStringLiteral( "checkPrimaryKeyUnicity" ), checkPrimaryKeyUnicity ? QLatin1Literal( "1" ) : QLatin1Literal( "0" ) );
395420

396421
QgsDebugMsg( QStringLiteral( "returning uri %1" ).arg( uri.uri( false ) ) );
397422
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: 0 additions & 1 deletion
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

‎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;
@@ -1323,7 +1324,7 @@ bool QgsPostgresProvider::determinePrimaryKey()
13231324

13241325
QStringList log;
13251326

1326-
// no primary or unique indizes found
1327+
// no primary or unique indices found
13271328
if ( res.PQntuples() == 0 )
13281329
{
13291330
QgsDebugMsg( QStringLiteral( "Relation has no primary key -- investigating alternatives" ) );
@@ -1601,7 +1602,6 @@ bool QgsPostgresProvider::uniqueData( const QString &quotedColNames )
16011602
pushError( unique.PQresultErrorMessage() );
16021603
return false;
16031604
}
1604-
16051605
return unique.PQntuples() == 1 && unique.PQgetvalue( 0, 0 ).startsWith( 't' );
16061606
}
16071607

‎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>777</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>543</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>178</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>-975</y>
1579-
<width>671</width>
1580-
<height>2682</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)
Please sign in to comment.