Skip to content
Permalink
Browse files

[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
  • Loading branch information
elpaso committed Apr 24, 2019
1 parent 51d8286 commit 38c91e4e9715321b0b1164eea3bca82b45eb830c
@@ -995,6 +995,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() );
@@ -1573,24 +1573,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;
@@ -1649,15 +1633,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" ) )
{
@@ -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 );

@@ -384,7 +398,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 );

@@ -402,7 +416,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 );
@@ -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(); }
@@ -1323,7 +1323,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" ) );
@@ -1601,7 +1601,6 @@ bool QgsPostgresProvider::uniqueData( const QString &quotedColNames )
pushError( unique.PQresultErrorMessage() );
return false;
}

return unique.PQntuples() == 1 && unique.PQgetvalue( 0, 0 ).startsWith( 't' );
}

@@ -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">
@@ -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)
@@ -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;

0 comments on commit 38c91e4

Please sign in to comment.
You can’t perform that action at this time.