Skip to content

Commit 38c91e4

Browse files
elpasoelpaso
committedApr 24, 2019
[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
·
ltr-3_40final-3_8_0
1 parent 51d8286 commit 38c91e4

File tree

8 files changed

+88
-46
lines changed

8 files changed

+88
-46
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: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,24 +1573,8 @@ QString QgsVectorLayer::loadDefaultStyle( bool &resultFlag )
15731573
bool QgsVectorLayer::setDataProvider( QString const &provider, const QgsDataProvider::ProviderOptions &options )
15741574
{
15751575
mProviderKey = provider;
1576-
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;
1581-
if ( provider.compare( QLatin1String( "postgres" ) ) == 0 )
1582-
{
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 );
1590-
}
1591-
15921576
delete mDataProvider;
1593-
mDataProvider = qobject_cast<QgsVectorDataProvider *>( QgsProviderRegistry::instance()->createProvider( provider, dataSource, options ) );
1577+
mDataProvider = qobject_cast<QgsVectorDataProvider *>( QgsProviderRegistry::instance()->createProvider( provider, mDataSource, options ) );
15941578
if ( !mDataProvider )
15951579
{
15961580
mValid = false;
@@ -1649,15 +1633,7 @@ bool QgsVectorLayer::setDataProvider( QString const &provider, const QgsDataProv
16491633
if ( !lName.isEmpty() )
16501634
setName( lName );
16511635
}
1652-
16531636
QgsDebugMsgLevel( QStringLiteral( "Beautified layer name %1" ).arg( name() ), 3 );
1654-
1655-
// deal with unnecessary schema qualification to make v.in.ogr happy
1656-
// and remove unnecessary key
1657-
QgsDataSourceUri dataProviderUri( mDataProvider->dataSourceUri() );
1658-
if ( dataProviderUri.hasParam( checkUnicityKey ) )
1659-
dataProviderUri.removeParam( checkUnicityKey );
1660-
mDataSource = dataProviderUri.uri( false );
16611637
}
16621638
else if ( mProviderKey == QLatin1String( "osm" ) )
16631639
{

‎src/providers/postgres/qgspgtablemodel.cpp

Lines changed: 19 additions & 5 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

@@ -39,7 +40,7 @@ QgsPgTableModel::QgsPgTableModel()
3940
headerLabels << tr( "Sql" );
4041
setHorizontalHeaderLabels( headerLabels );
4142
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 );
42-
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 );
43+
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 );
4344
}
4445

4546
void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProperty )
@@ -127,8 +128,21 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
127128

128129
QStandardItem *checkPkUnicityItem = new QStandardItem( QString() );
129130
checkPkUnicityItem->setFlags( checkPkUnicityItem->flags() | Qt::ItemIsUserCheckable );
130-
checkPkUnicityItem->setCheckState( Qt::Unchecked );
131-
checkPkUnicityItem->setToolTip( headerData( 9, Qt::Orientation::Horizontal, Qt::ToolTipRole ).toString() );
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( 9, 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+
}
132146

133147
QStandardItem *sqlItem = new QStandardItem( layerProperty.sql );
134148

@@ -384,7 +398,7 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
384398

385399
bool selectAtId = itemFromIndex( index.sibling( index.row(), DbtmSelectAtId ) )->checkState() == Qt::Checked;
386400
QString sql = index.sibling( index.row(), DbtmSql ).data( Qt::DisplayRole ).toString();
387-
bool checkPkUnicity = itemFromIndex( index.sibling( index.row(), DbtmCheckPkUnicity ) )->checkState() == Qt::Checked;
401+
bool checkPrimaryKeyUnicity = itemFromIndex( index.sibling( index.row(), DbtmCheckPkUnicity ) )->checkState() == Qt::Checked;
388402

389403
QgsDataSourceUri uri( connInfo );
390404

@@ -402,7 +416,7 @@ QString QgsPgTableModel::layerURI( const QModelIndex &index, const QString &conn
402416
uri.setWkbType( wkbType );
403417
uri.setSrid( srid );
404418
uri.disableSelectAtId( !selectAtId );
405-
uri.setParam( QStringLiteral( "checkPrimaryKeyUnicity" ), QString( checkPkUnicity ) );
419+
uri.setParam( QStringLiteral( "checkPrimaryKeyUnicity" ), checkPrimaryKeyUnicity ? QLatin1Literal( "1" ) : QLatin1Literal( "0" ) );
406420

407421
QgsDebugMsg( QStringLiteral( "returning uri %1" ).arg( uri.uri( false ) ) );
408422
return uri.uri( false );

‎src/providers/postgres/qgspostgresconn.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ struct QgsPostgresLayerProperty
8282
bool isView = false;
8383
bool isMaterializedView = false;
8484
QString tableComment;
85-
bool checkPkUnicity = false;
86-
8785

8886
// TODO: rename this !
8987
int size() const { Q_ASSERT( types.size() == srids.size() ); return types.size(); }

‎src/providers/postgres/qgspostgresprovider.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,7 @@ bool QgsPostgresProvider::determinePrimaryKey()
13231323

13241324
QStringList log;
13251325

1326-
// no primary or unique indizes found
1326+
// no primary or unique indices found
13271327
if ( res.PQntuples() == 0 )
13281328
{
13291329
QgsDebugMsg( QStringLiteral( "Relation has no primary key -- investigating alternatives" ) );
@@ -1601,7 +1601,6 @@ bool QgsPostgresProvider::uniqueData( const QString &quotedColNames )
16011601
pushError( unique.PQresultErrorMessage() );
16021602
return false;
16031603
}
1604-
16051604
return unique.PQntuples() == 1 && unique.PQgetvalue( 0, 0 ).startsWith( 't' );
16061605
}
16071606

‎src/ui/qgsprojectpropertiesbase.ui

Lines changed: 12 additions & 12 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>671</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>685</width>
867+
<height>780</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>685</width>
942+
<height>780</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 Postgres 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>
1578+
<y>0</y>
15791579
<width>671</width>
1580-
<height>2682</height>
1580+
<height>2732</height>
15811581
</rect>
15821582
</property>
15831583
<layout class="QVBoxLayout" name="verticalLayout_13">

‎tests/src/python/test_provider_postgres.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,21 @@ 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+
# This is NOT valid
1108+
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')
1109+
self.assertFalse(vl0.isValid())
1110+
1111+
# This is valid because the default is to not check unicity
1112+
vl0 = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'an_int\' srid=0 type=POINT table="qgis_test"."b21839_pk_unicity_view" (geom) sql=', 'test', 'postgres')
1113+
self.assertFalse(vl0.isValid())
1114+
11001115
def testNotify(self):
11011116
vl0 = QgsVectorLayer(self.dbconn + ' sslmode=disable key=\'pk\' srid=4326 type=POLYGON table="qgis_test"."some_poly_data" (geom) sql=', 'test', 'postgres')
11021117
vl0.dataProvider().setListening(True)

‎tests/testdata/provider/testdata_pg.sql

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,3 +535,41 @@ INSERT INTO qgis_test.check_constraints VALUES (
535535
4, -- a
536536
3 -- b
537537
)
538+
539+
540+
---------------------------------------------
541+
--
542+
-- Table and view for tests on checkPrimaryKeyUnicity
543+
--
544+
545+
546+
DROP TABLE IF EXISTS qgis_test.b21839_pk_unicity CASCADE;
547+
CREATE TABLE qgis_test.b21839_pk_unicity
548+
(
549+
pk serial NOT NULL,
550+
an_int integer NOT NULL,
551+
geom geometry(Point),
552+
CONSTRAINT b21839_pk_unicity_pkey PRIMARY KEY (pk)
553+
)
554+
WITH (
555+
OIDS=FALSE
556+
);
557+
558+
559+
INSERT INTO qgis_test.b21839_pk_unicity(
560+
pk, an_int, geom)
561+
VALUES (1, 1, ST_GeomFromText('point( 1 1)'));
562+
563+
564+
INSERT INTO qgis_test.b21839_pk_unicity(
565+
pk, an_int, geom)
566+
VALUES (2, 1, 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.geom
574+
FROM qgis_test.b21839_pk_unicity;
575+

0 commit comments

Comments
 (0)
Please sign in to comment.