From 1fea20de504204456d38cdcebf89e56f6dea0218 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 7 Nov 2016 14:27:17 +1000 Subject: [PATCH] Split handling of literal default values from provider side default value SQL clauses QgsVectorDataProvider now has two methods: - defaultValueClause: returns SQL fragment which must be evaluated by the provider to obtain the default value, eg sequence values - defaultValue: returns the literal constant default value for a field --- python/core/qgsvectordataprovider.sip | 15 ++++++++++-- src/app/qgisapp.cpp | 6 ++--- src/app/qgsfeatureaction.cpp | 2 +- src/app/qgsidentifyresultsdialog.cpp | 2 +- src/app/qgsmergeattributesdialog.cpp | 4 ++-- src/core/qgsofflineediting.cpp | 2 +- src/core/qgsvectordataprovider.cpp | 6 +++++ src/core/qgsvectordataprovider.h | 15 ++++++++++-- src/core/qgsvectorlayereditutils.cpp | 2 +- .../core/qgseditorwidgetfactory.cpp | 2 +- .../core/qgseditorwidgetwrapper.cpp | 2 +- src/providers/mssql/qgsmssqlprovider.cpp | 13 ++++------ src/providers/mssql/qgsmssqlprovider.h | 4 ++-- .../postgres/qgspostgresprovider.cpp | 24 ++++++++++++++----- src/providers/postgres/qgspostgresprovider.h | 1 + tests/src/python/test_provider_postgres.py | 23 ++++++++++++------ .../src/python/test_qgsrelationeditwidget.py | 4 ++-- 17 files changed, 87 insertions(+), 40 deletions(-) diff --git a/python/core/qgsvectordataprovider.sip b/python/core/qgsvectordataprovider.sip index 3983d5e0b34..ec2fdb56f79 100644 --- a/python/core/qgsvectordataprovider.sip +++ b/python/core/qgsvectordataprovider.sip @@ -226,9 +226,20 @@ class QgsVectorDataProvider : QgsDataProvider const QMap &geometry_map ); /** - * Returns the default value for field specified by @c fieldId + * Returns any literal default values which are present at the provider for a specified + * field index. + * @see defaultValueClause() */ - virtual QVariant defaultValue( int fieldId ) const; + virtual QVariant defaultValue( int fieldIndex ) const; + + /** + * Returns any default value clauses which are present at the provider for a specified + * field index. These clauses are usually SQL fragments which must be evaluated by the + * provider, eg sequence values. + * @see defaultValue() + * @note added in QGIS 3.0 + */ + virtual QString defaultValueClause( int fieldIndex ) const; /** * Returns any constraints which are present at the provider for a specified diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp index c1badc29266..bb3420536d6 100644 --- a/src/app/qgisapp.cpp +++ b/src/app/qgisapp.cpp @@ -6988,7 +6988,7 @@ void QgisApp::mergeAttributesOfSelectedFeatures() QgsField fld( vl->fields().at( i ) ); bool isDefaultValue = vl->fields().fieldOrigin( i ) == QgsFields::OriginProvider && vl->dataProvider() && - vl->dataProvider()->defaultValue( vl->fields().fieldOriginIndex( i ) ) == val; + vl->dataProvider()->defaultValueClause( vl->fields().fieldOriginIndex( i ) ) == val; // convert to destination data type if ( !isDefaultValue && !fld.convertCompatible( val ) ) @@ -7167,7 +7167,7 @@ void QgisApp::mergeSelectedFeatures() QVariant val = attrs.at( i ); bool isDefaultValue = vl->fields().fieldOrigin( i ) == QgsFields::OriginProvider && vl->dataProvider() && - vl->dataProvider()->defaultValue( vl->fields().fieldOriginIndex( i ) ) == val; + vl->dataProvider()->defaultValueClause( vl->fields().fieldOriginIndex( i ) ) == val; // convert to destination data type if ( !isDefaultValue && !vl->fields().at( i ).convertCompatible( val ) ) @@ -7481,7 +7481,7 @@ void QgisApp::editPaste( QgsMapLayer *destinationLayer ) } else { - defVal = pasteVectorLayer->dataProvider()->defaultValue( dst ); + defVal = pasteVectorLayer->dataProvider()->defaultValueClause( dst ); } if ( defVal.isValid() && !defVal.isNull() ) diff --git a/src/app/qgsfeatureaction.cpp b/src/app/qgsfeatureaction.cpp index c39dbd33f30..9ebf573ed6e 100644 --- a/src/app/qgsfeatureaction.cpp +++ b/src/app/qgsfeatureaction.cpp @@ -176,7 +176,7 @@ bool QgsFeatureAction::addFeature( const QgsAttributeMap& defaultAttributes, boo } else { - v = provider->defaultValue( idx ); + v = provider->defaultValueClause( idx ); } mFeature->setAttribute( idx, v ); diff --git a/src/app/qgsidentifyresultsdialog.cpp b/src/app/qgsidentifyresultsdialog.cpp index d8da668f364..88506484757 100644 --- a/src/app/qgsidentifyresultsdialog.cpp +++ b/src/app/qgsidentifyresultsdialog.cpp @@ -489,7 +489,7 @@ void QgsIdentifyResultsDialog::addFeature( QgsVectorLayer *vlayer, const QgsFeat QString defVal; if ( fields.fieldOrigin( i ) == QgsFields::OriginProvider && vlayer->dataProvider() ) - defVal = vlayer->dataProvider()->defaultValue( fields.fieldOriginIndex( i ) ).toString(); + defVal = vlayer->dataProvider()->defaultValueClause( fields.fieldOriginIndex( i ) ); QString value = defVal == attrs.at( i ) ? defVal : fields.at( i ).displayString( attrs.at( i ) ); QgsTreeWidgetItem *attrItem = new QgsTreeWidgetItem( QStringList() << QString::number( i ) << value ); diff --git a/src/app/qgsmergeattributesdialog.cpp b/src/app/qgsmergeattributesdialog.cpp index de37fa0cb86..e04b586c36d 100644 --- a/src/app/qgsmergeattributesdialog.cpp +++ b/src/app/qgsmergeattributesdialog.cpp @@ -540,7 +540,7 @@ QgsAttributes QgsMergeAttributesDialog::mergedAttributes() const if ( !mVectorLayer->defaultValueExpression( fieldIdx ).isEmpty() ) results[fieldIdx] = mVectorLayer->defaultValue( fieldIdx, mFeatureList.at( 0 ), &context ); else if ( mVectorLayer->dataProvider() ) - results[fieldIdx] = mVectorLayer->dataProvider()->defaultValue( fieldIdx ); + results[fieldIdx] = mVectorLayer->dataProvider()->defaultValueClause( fieldIdx ); else results[fieldIdx] = QVariant(); continue; @@ -567,7 +567,7 @@ QgsAttributes QgsMergeAttributesDialog::mergedAttributes() const } else if ( mVectorLayer->dataProvider() ) { - results[fieldIdx] = mVectorLayer->dataProvider()->defaultValue( fieldIdx ); + results[fieldIdx] = mVectorLayer->dataProvider()->defaultValueClause( fieldIdx ); } widgetIndex++; } diff --git a/src/core/qgsofflineediting.cpp b/src/core/qgsofflineediting.cpp index 6bd50c33b0d..8cf7d115959 100644 --- a/src/core/qgsofflineediting.cpp +++ b/src/core/qgsofflineediting.cpp @@ -793,7 +793,7 @@ void QgsOfflineEditing::applyFeaturesAdded( QgsVectorLayer* offlineLayer, QgsVec if ( !remoteLayer->defaultValueExpression( k ).isEmpty() ) newAttrs[k] = remoteLayer->defaultValue( k, f, &context ); else if ( remoteFlds.fieldOrigin( k ) == QgsFields::OriginProvider ) - newAttrs[k] = remoteLayer->dataProvider()->defaultValue( remoteFlds.fieldOriginIndex( k ) ); + newAttrs[k] = remoteLayer->dataProvider()->defaultValueClause( remoteFlds.fieldOriginIndex( k ) ); } f.setAttributes( newAttrs ); diff --git a/src/core/qgsvectordataprovider.cpp b/src/core/qgsvectordataprovider.cpp index d84f7a1604d..2b50b78e299 100644 --- a/src/core/qgsvectordataprovider.cpp +++ b/src/core/qgsvectordataprovider.cpp @@ -98,6 +98,12 @@ QVariant QgsVectorDataProvider::defaultValue( int fieldId ) const return QVariant(); } +QString QgsVectorDataProvider::defaultValueClause( int fieldIndex ) const +{ + Q_UNUSED( fieldIndex ); + return QString(); +} + QgsFieldConstraints::Constraints QgsVectorDataProvider::fieldConstraints( int fieldIndex ) const { QgsFields f = fields(); diff --git a/src/core/qgsvectordataprovider.h b/src/core/qgsvectordataprovider.h index d6b78b157ba..9126b424743 100644 --- a/src/core/qgsvectordataprovider.h +++ b/src/core/qgsvectordataprovider.h @@ -277,9 +277,20 @@ class CORE_EXPORT QgsVectorDataProvider : public QgsDataProvider const QgsGeometryMap &geometry_map ); /** - * Returns the default value for field specified by @c fieldId + * Returns any literal default values which are present at the provider for a specified + * field index. + * @see defaultValueClause() */ - virtual QVariant defaultValue( int fieldId ) const; + virtual QVariant defaultValue( int fieldIndex ) const; + + /** + * Returns any default value clauses which are present at the provider for a specified + * field index. These clauses are usually SQL fragments which must be evaluated by the + * provider, eg sequence values. + * @see defaultValue() + * @note added in QGIS 3.0 + */ + virtual QString defaultValueClause( int fieldIndex ) const; /** * Returns any constraints which are present at the provider for a specified diff --git a/src/core/qgsvectorlayereditutils.cpp b/src/core/qgsvectorlayereditutils.cpp index cb4962bb53f..8a12b9372f8 100644 --- a/src/core/qgsvectorlayereditutils.cpp +++ b/src/core/qgsvectorlayereditutils.cpp @@ -381,7 +381,7 @@ int QgsVectorLayerEditUtils::splitFeatures( const QList& splitLine, bo QgsAttributes newAttributes = feat.attributes(); Q_FOREACH ( int pkIdx, L->dataProvider()->pkAttributeIndexes() ) { - const QVariant defaultValue = L->dataProvider()->defaultValue( pkIdx ); + const QVariant defaultValue = L->dataProvider()->defaultValueClause( pkIdx ); if ( !defaultValue.isNull() ) { newAttributes[ pkIdx ] = defaultValue; diff --git a/src/gui/editorwidgets/core/qgseditorwidgetfactory.cpp b/src/gui/editorwidgets/core/qgseditorwidgetfactory.cpp index 8d2d584c1ad..a78e0371db5 100644 --- a/src/gui/editorwidgets/core/qgseditorwidgetfactory.cpp +++ b/src/gui/editorwidgets/core/qgseditorwidgetfactory.cpp @@ -70,7 +70,7 @@ QString QgsEditorWidgetFactory::representValue( QgsVectorLayer* vl, int fieldIdx QString defVal; if ( vl->fields().fieldOrigin( fieldIdx ) == QgsFields::OriginProvider && vl->dataProvider() ) - defVal = vl->dataProvider()->defaultValue( vl->fields().fieldOriginIndex( fieldIdx ) ).toString(); + defVal = vl->dataProvider()->defaultValueClause( vl->fields().fieldOriginIndex( fieldIdx ) ); return value == defVal ? defVal : vl->fields().at( fieldIdx ).displayString( value ); } diff --git a/src/gui/editorwidgets/core/qgseditorwidgetwrapper.cpp b/src/gui/editorwidgets/core/qgseditorwidgetwrapper.cpp index d12381f95d2..54de6ac8505 100644 --- a/src/gui/editorwidgets/core/qgseditorwidgetwrapper.cpp +++ b/src/gui/editorwidgets/core/qgseditorwidgetwrapper.cpp @@ -44,7 +44,7 @@ QgsField QgsEditorWidgetWrapper::field() const QVariant QgsEditorWidgetWrapper::defaultValue() const { - mDefaultValue = layer()->dataProvider()->defaultValue( mFieldIdx ); + mDefaultValue = layer()->dataProvider()->defaultValueClause( mFieldIdx ); return mDefaultValue; } diff --git a/src/providers/mssql/qgsmssqlprovider.cpp b/src/providers/mssql/qgsmssqlprovider.cpp index e5fc331968d..d0cbd1f564e 100644 --- a/src/providers/mssql/qgsmssqlprovider.cpp +++ b/src/providers/mssql/qgsmssqlprovider.cpp @@ -433,7 +433,7 @@ void QgsMssqlProvider::loadFields() if ( !query.value( 12 ).isNull() ) { - mDefaultValues.insert( i, query.value( 12 ) ); + mDefaultValues.insert( i, query.value( 12 ).toString() ); } ++i; } @@ -501,12 +501,9 @@ QString QgsMssqlProvider::quotedValue( const QVariant& value ) } } -QVariant QgsMssqlProvider::defaultValue( int fieldId ) const +QString QgsMssqlProvider::defaultValueClause( int fieldId ) const { - if ( mDefaultValues.contains( fieldId ) ) - return mDefaultValues[fieldId]; - else - return QVariant( QString::null ); + return mDefaultValues.value( fieldId, QString() ); } QString QgsMssqlProvider::storageType() const @@ -815,7 +812,7 @@ bool QgsMssqlProvider::addFeatures( QgsFeatureList & flist ) if ( fld.name().isEmpty() ) continue; // invalid - if ( mDefaultValues.contains( i ) && mDefaultValues[i] == attrs.at( i ) ) + if ( mDefaultValues.contains( i ) && mDefaultValues.value( i ) == attrs.at( i ).toString() ) continue; // skip fields having default values if ( !first ) @@ -886,7 +883,7 @@ bool QgsMssqlProvider::addFeatures( QgsFeatureList & flist ) if ( fld.name().isEmpty() ) continue; // invalid - if ( mDefaultValues.contains( i ) && mDefaultValues[i] == attrs.at( i ) ) + if ( mDefaultValues.contains( i ) && mDefaultValues.value( i ) == attrs.at( i ).toString() ) continue; // skip fields having default values QVariant::Type type = fld.type(); diff --git a/src/providers/mssql/qgsmssqlprovider.h b/src/providers/mssql/qgsmssqlprovider.h index 916cf6fc06a..e995c1c9988 100644 --- a/src/providers/mssql/qgsmssqlprovider.h +++ b/src/providers/mssql/qgsmssqlprovider.h @@ -186,7 +186,7 @@ class QgsMssqlProvider : public QgsVectorDataProvider //! Convert values to quoted values for database work * static QString quotedValue( const QVariant& value ); - QVariant defaultValue( int fieldId ) const override; + QString defaultValueClause( int fieldId ) const override; //! Import a vector layer into the database static QgsVectorLayerImport::ImportError createEmptyLayer( @@ -212,7 +212,7 @@ class QgsMssqlProvider : public QgsVectorDataProvider //! Fields QgsFields mAttributeFields; - QMap mDefaultValues; + QMap mDefaultValues; mutable QgsMssqlGeometryParser mParser; diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp index 8c35fc122bb..5e8da6a94d6 100644 --- a/src/providers/postgres/qgspostgresprovider.cpp +++ b/src/providers/postgres/qgspostgresprovider.cpp @@ -1716,15 +1716,27 @@ bool QgsPostgresProvider::isValid() const return mValid; } +QString QgsPostgresProvider::defaultValueClause( int fieldId ) const +{ + QString defVal = mDefaultValues.value( fieldId, QString() ); + + if ( !providerProperty( EvaluateDefaultValues, false ).toBool() && !defVal.isEmpty() ) + { + return defVal; + } + + return QString(); +} + QVariant QgsPostgresProvider::defaultValue( int fieldId ) const { - QVariant defVal = mDefaultValues.value( fieldId, QString::null ); + QString defVal = mDefaultValues.value( fieldId, QString() ); - if ( providerProperty( EvaluateDefaultValues, false ).toBool() && !defVal.isNull() ) + if ( providerProperty( EvaluateDefaultValues, false ).toBool() && !defVal.isEmpty() ) { QgsField fld = field( fieldId ); - QgsPostgresResult res( connectionRO()->PQexec( QStringLiteral( "SELECT %1" ).arg( defVal.toString() ) ) ); + QgsPostgresResult res( connectionRO()->PQexec( QStringLiteral( "SELECT %1" ).arg( defVal ) ) ); if ( res.result() ) return convertValue( fld.type(), fld.subType(), res.PQgetvalue( 0, 0 ) ); @@ -1735,7 +1747,7 @@ QVariant QgsPostgresProvider::defaultValue( int fieldId ) const } } - return defVal; + return QVariant(); } QString QgsPostgresProvider::paramValue( const QString& fieldValue, const QString &defaultValue ) const @@ -1891,7 +1903,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist ) values += delim + QStringLiteral( "$%1" ).arg( defaultValues.size() + offset ); delim = ','; fieldId << idx; - defaultValues << defaultValue( idx ).toString(); + defaultValues << defaultValueClause( idx ); } } @@ -1928,7 +1940,7 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist ) insert += delim + quotedIdentifier( fieldname ); - QString defVal = defaultValue( idx ).toString(); + QString defVal = defaultValueClause( idx ); if ( i == flist.size() ) { diff --git a/src/providers/postgres/qgspostgresprovider.h b/src/providers/postgres/qgspostgresprovider.h index eb90e2d770d..6245b20535d 100644 --- a/src/providers/postgres/qgspostgresprovider.h +++ b/src/providers/postgres/qgspostgresprovider.h @@ -160,6 +160,7 @@ class QgsPostgresProvider : public QgsVectorDataProvider virtual bool isSaveAndLoadStyleToDBSupported() const override { return true; } QgsAttributeList attributeIndexes() const override; QgsAttributeList pkAttributeIndexes() const override { return mPrimaryKeyAttrs; } + QString defaultValueClause( int fieldId ) const override; QVariant defaultValue( int fieldId ) const override; /** Adds a list of features diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py index 18ee6341b38..97ddb8ab3f5 100644 --- a/tests/src/python/test_provider_postgres.py +++ b/tests/src/python/test_provider_postgres.py @@ -28,6 +28,7 @@ from qgis.core import ( QgsTransactionGroup, QgsField, QgsFieldConstraints, + QgsDataProvider, NULL ) from qgis.gui import QgsEditorWidgetRegistry @@ -84,9 +85,17 @@ class TestPyQgsPostgresProvider(unittest.TestCase, ProviderTestCase): # HERE GO THE PROVIDER SPECIFIC TESTS def testDefaultValue(self): - self.assertEqual(self.provider.defaultValue(0), 'nextval(\'qgis_test."someData_pk_seq"\'::regclass)') + self.provider.setProviderProperty(QgsDataProvider.EvaluateDefaultValues, True) + self.assertTrue(isinstance(self.provider.defaultValue(0), int)) self.assertEqual(self.provider.defaultValue(1), NULL) - self.assertEqual(self.provider.defaultValue(2), '\'qgis\'::text') + self.assertEqual(self.provider.defaultValue(2), 'qgis') + self.provider.setProviderProperty(QgsDataProvider.EvaluateDefaultValues, False) + + def testDefaultValueClause(self): + self.provider.setProviderProperty(QgsDataProvider.EvaluateDefaultValues, False) + self.assertEqual(self.provider.defaultValueClause(0), 'nextval(\'qgis_test."someData_pk_seq"\'::regclass)') + self.assertFalse(self.provider.defaultValueClause(1)) + self.assertEqual(self.provider.defaultValueClause(2), '\'qgis\'::text') def testDateTimeTypes(self): vl = QgsVectorLayer('%s table="qgis_test"."date_times" sql=' % (self.dbconn), "testdatetimes", "postgres") @@ -248,7 +257,7 @@ class TestPyQgsPostgresProvider(unittest.TestCase, ProviderTestCase): vl = QgsVectorLayer('{} table="qgis_test"."{}" key="obj_id" sql='.format(self.dbconn, 'oid_serial_table'), "oid_serial", "postgres") self.assertTrue(vl.isValid()) f = QgsFeature(vl.fields()) - f['obj_id'] = vl.dataProvider().defaultValue(0) + f['obj_id'] = vl.dataProvider().defaultValueClause(0) f['name'] = 'Test' r, f = vl.dataProvider().addFeatures([f]) self.assertTrue(r) @@ -553,17 +562,17 @@ class TestPyQgsPostgresProvider(unittest.TestCase, ProviderTestCase): oflds = olyr.fields() if key is None: # if the pkey was not given, it will create a pkey - self.assertEquals(oflds.size(), flds.size() + 1) - self.assertEquals(oflds[0].name(), kfnames[0]) + self.assertEqual(oflds.size(), flds.size() + 1) + self.assertEqual(oflds[0].name(), kfnames[0]) for i in range(flds.size()): self.assertEqual(oflds[i + 1].name(), flds[i].name()) else: # pkey was given, no extra field generated - self.assertEquals(oflds.size(), flds.size()) + self.assertEqual(oflds.size(), flds.size()) for i in range(oflds.size()): self.assertEqual(oflds[i].name(), flds[i].name()) pks = olyr.pkAttributeList() - self.assertEquals(len(pks), len(kfnames)) + self.assertEqual(len(pks), len(kfnames)) for i in range(0, len(kfnames)): self.assertEqual(oflds[pks[i]].name(), kfnames[i]) diff --git a/tests/src/python/test_qgsrelationeditwidget.py b/tests/src/python/test_qgsrelationeditwidget.py index b6e1c84f97f..be4877519db 100644 --- a/tests/src/python/test_qgsrelationeditwidget.py +++ b/tests/src/python/test_qgsrelationeditwidget.py @@ -153,7 +153,7 @@ class TestQgsRelationEditWidget(unittest.TestCase): wrapper = self.createWrapper(self.vl_a, '"name"=\'Douglas Adams\'') # NOQA f = QgsFeature(self.vl_b.fields()) - f.setAttributes([self.vl_b.dataProvider().defaultValue(0), 'The Hitchhiker\'s Guide to the Galaxy']) + f.setAttributes([self.vl_b.dataProvider().defaultValueClause(0), 'The Hitchhiker\'s Guide to the Galaxy']) self.vl_b.addFeature(f) def choose_linked_feature(): @@ -318,7 +318,7 @@ class VlTools(QgsVectorLayerTools): if v: values.append(v) else: - values.append(layer.dataProvider().defaultValue(i)) + values.append(layer.dataProvider().defaultValueClause(i)) f = QgsFeature(layer.fields()) f.setAttributes(self.values) f.setGeometry(defaultGeometry)