Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[postgresql] Fix checkPrimaryKeyUnicity option
Cherry picked from release-3_6 and master 4da0413
  • Loading branch information
elpaso committed Apr 29, 2019
1 parent 8131f8c commit e75681d
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 44 deletions.
2 changes: 2 additions & 0 deletions src/app/qgsprojectproperties.cpp
Expand Up @@ -950,6 +950,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() );
Expand Down
37 changes: 15 additions & 22 deletions src/core/qgsvectorlayer.cpp
Expand Up @@ -1572,25 +1572,26 @@ QString QgsVectorLayer::loadDefaultStyle( bool &resultFlag )

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

// 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;
// For Postgres provider primary key unicity is tested at construction time,
// so it has to be set before initializing the provider,
// this manipulation is necessary to preserve default behavior when
// "trust layer metadata" project level option is set and checkPrimaryKeyUnicity
// was not explicitly passed in the uri
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 );
const QString checkUnicityKey { QStringLiteral( "checkPrimaryKeyUnicity" ) };
QgsDataSourceUri uri( mDataSource );
if ( ! uri.hasParam( checkUnicityKey ) )
{
uri.setParam( checkUnicityKey, mReadExtentFromXml ? "0" : "1" );
mDataSource = 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 )
{
QgsDebugMsgLevel( QStringLiteral( "Unable to get data provider" ), 2 );
Expand Down Expand Up @@ -1648,15 +1649,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" ) )
{
Expand Down
30 changes: 28 additions & 2 deletions src/providers/postgres/qgspgtablemodel.cpp
Expand Up @@ -19,6 +19,8 @@
#include "qgsdataitem.h"
#include "qgslogger.h"
#include "qgsapplication.h"
#include "qgssettings.h"
#include "qgsproject.h"

#include <climits>

Expand All @@ -34,8 +36,11 @@ QgsPgTableModel::QgsPgTableModel()
headerLabels << tr( "SRID" );
headerLabels << tr( "Feature id" );
headerLabels << tr( "Select at id" );
headerLabels << tr( "Check PK unicity" );
headerLabels << tr( "Sql" );
setHorizontalHeaderLabels( headerLabels );
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 );
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 );
}

void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProperty )
Expand Down Expand Up @@ -116,7 +121,25 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
QStandardItem *selItem = new QStandardItem( QString() );
selItem->setFlags( selItem->flags() | Qt::ItemIsUserCheckable );
selItem->setCheckState( Qt::Checked );
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)." ) );
selItem->setToolTip( headerData( Columns::DbtmSelectAtId, Qt::Orientation::Horizontal, Qt::ToolTipRole ).toString() );

QStandardItem *checkPkUnicityItem = new QStandardItem( QString() );
checkPkUnicityItem->setFlags( checkPkUnicityItem->flags() | Qt::ItemIsUserCheckable );

// 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( Columns::DbtmCheckPkUnicity, 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 );

Expand All @@ -131,6 +154,7 @@ void QgsPgTableModel::addTableEntry( const QgsPostgresLayerProperty &layerProper
childItemList << sridItem;
childItemList << pkItem;
childItemList << selItem;
childItemList << checkPkUnicityItem;
childItemList << sqlItem;

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

if ( tip.isEmpty() )
if ( tip.isEmpty() && item != checkPkUnicityItem && item != selItem )
{
item->setToolTip( QString() );
}
Expand Down Expand Up @@ -370,6 +394,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 checkPrimaryKeyUnicity = itemFromIndex( index.sibling( index.row(), DbtmCheckPkUnicity ) )->checkState() == Qt::Checked;

QgsDataSourceUri uri( connInfo );

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

QgsDebugMsg( QStringLiteral( "returning uri %1" ).arg( uri.uri( false ) ) );
return uri.uri( false );
Expand Down
1 change: 1 addition & 0 deletions src/providers/postgres/qgspgtablemodel.h
Expand Up @@ -53,6 +53,7 @@ class QgsPgTableModel : public QStandardItemModel
DbtmSrid,
DbtmPkCol,
DbtmSelectAtId,
DbtmCheckPkUnicity,
DbtmSql,
DbtmColumns
};
Expand Down
13 changes: 8 additions & 5 deletions src/providers/postgres/qgspostgresconn.h
Expand Up @@ -83,7 +83,6 @@ struct QgsPostgresLayerProperty
bool isMaterializedView = false;
QString tableComment;


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

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

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

//! PQgetResult is used for asynchronous queries (with PQsendQuery)
//! Thread safety must be ensured by the caller by calling QgsPostgresConn::lock() and QgsPostgresConn::unlock()
/**
* PQgetResult is used for asynchronous queries (with PQsendQuery)
* Thread safety must be ensured by the caller by calling QgsPostgresConn::lock() and QgsPostgresConn::unlock()
*/
PGresult *PQgetResult();

bool begin();
Expand Down
4 changes: 2 additions & 2 deletions src/providers/postgres/qgspostgresprovider.cpp
Expand Up @@ -108,6 +108,7 @@ QgsPostgresProvider::QgsPostgresProvider( QString const &uri, const ProviderOpti

if ( mUri.hasParam( QStringLiteral( "checkPrimaryKeyUnicity" ) ) )
{

if ( mUri.param( QStringLiteral( "checkPrimaryKeyUnicity" ) ).compare( QLatin1String( "0" ) ) == 0 )
{
mCheckPrimaryKeyUnicity = false;
Expand Down Expand Up @@ -1319,7 +1320,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" ) );
Expand Down Expand Up @@ -1575,7 +1576,6 @@ bool QgsPostgresProvider::uniqueData( const QString &quotedColNames )
pushError( unique.PQresultErrorMessage() );
return false;
}

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

Expand Down
26 changes: 13 additions & 13 deletions src/ui/qgsprojectpropertiesbase.ui
Expand Up @@ -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">
Expand Down Expand Up @@ -265,8 +265,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>590</width>
<height>782</height>
<width>537</width>
<height>795</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_8">
Expand Down Expand Up @@ -863,8 +863,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>603</width>
<height>161</height>
<width>546</width>
<height>164</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_7">
Expand Down Expand Up @@ -938,8 +938,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>290</width>
<height>552</height>
<width>269</width>
<height>553</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_12">
Expand Down Expand Up @@ -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 PostgreSQL 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>
Expand Down Expand Up @@ -1514,8 +1514,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>168</width>
<height>54</height>
<width>167</width>
<height>55</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_17">
Expand Down Expand Up @@ -1575,9 +1575,9 @@
<property name="geometry">
<rect>
<x>0</x>
<y>-1053</y>
<width>671</width>
<height>2645</height>
<y>0</y>
<width>598</width>
<height>2732</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_13">
Expand Down
31 changes: 31 additions & 0 deletions tests/src/python/test_provider_postgres.py
Expand Up @@ -1097,6 +1097,37 @@ 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())

geom = vl0.getFeature(1).geometry().asWkt()

# 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 NOT valid because the default is to 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())

# This is valid because the readExtentFromXml option is set
options = QgsVectorLayer.LayerOptions(True, True) # loadDefaultStyle, readExtentFromXml
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)
self.assertTrue(vl0.isValid())

# Valid because a_unique_int is unique and default is to check unicity
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')
self.assertEqual(vl0.getFeature(1).geometry().asWkt(), geom)

# Valid because a_unique_int is unique
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')
self.assertTrue(vl0.isValid())
self.assertEqual(vl0.getFeature(1).geometry().asWkt(), geom)

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)
Expand Down
39 changes: 39 additions & 0 deletions tests/testdata/provider/testdata_pg.sql
Expand Up @@ -534,4 +534,43 @@ INSERT INTO qgis_test.check_constraints VALUES (
1, -- id
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,
a_unique_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, a_unique_int , geom)
VALUES (1, 1, 1, ST_GeomFromText('point( 1 1)'));


INSERT INTO qgis_test.b21839_pk_unicity(
pk, an_int, a_unique_int, geom)
VALUES (2, 1, 2, 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.a_unique_int,
b21839_pk_unicity.geom
FROM qgis_test.b21839_pk_unicity;

0 comments on commit e75681d

Please sign in to comment.