Avoid double-evaluating postgres default values

This commit is contained in:
Nyall Dawson 2016-11-14 10:07:52 +10:00 committed by Matthias Kuhn
parent 249c8dca20
commit 60bbd09339
7 changed files with 102 additions and 13 deletions

View File

@ -239,7 +239,13 @@ class QgsVectorDataProvider : QgsDataProvider
/**
* Returns any literal default values which are present at the provider for a specified
* field index.
* field index. Important - this should ONLY be called when creating an attribute to insert
* directly into the database. Do not call this method for non-feature creation or modification,
* eg when validating an attribute or to compare it against an existing value, as calling it
* can cause changes to the underlying data source (eg Postgres provider where the default value
* is calculated as a result of a sequence). It is recommended that you instead use the methods
* in QgsVectorLayerUtils such as QgsVectorLayerUtils::createFeature()
* so that default value handling and validation is automatically carried out.
* @see defaultValueClause()
*/
virtual QVariant defaultValue( int fieldIndex ) const;
@ -257,9 +263,19 @@ class QgsVectorDataProvider : QgsDataProvider
* Returns any constraints which are present at the provider for a specified
* field index.
* @note added in QGIS 3.0
* @see skipConstraintCheck()
*/
QgsFieldConstraints::Constraints fieldConstraints( int fieldIndex ) const;
/**
* Returns true if a constraint check should be skipped for a specified field (eg if
* the value returned by defaultValue() is trusted implicitly. An optional attribute value can be
* passed which can help refine the skip constraint check.
* @note added in QGIS 3.0
* @see fieldConstraints()
*/
virtual bool skipConstraintCheck( int fieldIndex, QgsFieldConstraints::Constraint constraint, const QVariant& value = QVariant() ) const;
/**
* Changes geometries of existing features
* @param geometry_map A QgsGeometryMap whose index contains the feature IDs

View File

@ -114,6 +114,11 @@ QgsFieldConstraints::Constraints QgsVectorDataProvider::fieldConstraints( int fi
return f.at( fieldIndex ).constraints().constraints();
}
bool QgsVectorDataProvider::skipConstraintCheck( int, QgsFieldConstraints::Constraint, const QVariant& ) const
{
return false;
}
bool QgsVectorDataProvider::changeGeometryValues( const QgsGeometryMap &geometry_map )
{
Q_UNUSED( geometry_map );

View File

@ -293,7 +293,13 @@ class CORE_EXPORT QgsVectorDataProvider : public QgsDataProvider
/**
* Returns any literal default values which are present at the provider for a specified
* field index.
* field index. Important - this should ONLY be called when creating an attribute to insert
* directly into the database. Do not call this method for non-feature creation or modification,
* eg when validating an attribute or to compare it against an existing value, as calling it
* can cause changes to the underlying data source (eg Postgres provider where the default value
* is calculated as a result of a sequence). It is recommended that you instead use the methods
* in QgsVectorLayerUtils such as QgsVectorLayerUtils::createFeature()
* so that default value handling and validation is automatically carried out.
* @see defaultValueClause()
*/
virtual QVariant defaultValue( int fieldIndex ) const;
@ -311,9 +317,19 @@ class CORE_EXPORT QgsVectorDataProvider : public QgsDataProvider
* Returns any constraints which are present at the provider for a specified
* field index.
* @note added in QGIS 3.0
* @see skipConstraintCheck()
*/
QgsFieldConstraints::Constraints fieldConstraints( int fieldIndex ) const;
/**
* Returns true if a constraint check should be skipped for a specified field (eg if
* the value returned by defaultValue() is trusted implicitly. An optional attribute value can be
* passed which can help refine the skip constraint check.
* @note added in QGIS 3.0
* @see fieldConstraints()
*/
virtual bool skipConstraintCheck( int fieldIndex, QgsFieldConstraints::Constraint constraint, const QVariant& value = QVariant() ) const;
/**
* Changes geometries of existing features
* @param geometry_map A QgsGeometryMap whose index contains the feature IDs

View File

@ -143,7 +143,8 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
if ( attributeIndex < 0 || attributeIndex >= layer->fields().count() )
return false;
QgsField field = layer->fields().at( attributeIndex );
QgsFields fields = layer->fields();
QgsField field = fields.at( attributeIndex );
QVariant value = feature.attribute( attributeIndex );
bool valid = true;
errors.clear();
@ -179,11 +180,22 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
&& ( strength == QgsFieldConstraints::ConstraintStrengthNotSet || strength == constraints.constraintStrength( QgsFieldConstraints::ConstraintNotNull ) )
&& ( origin == QgsFieldConstraints::ConstraintOriginNotSet || origin == constraints.constraintOrigin( QgsFieldConstraints::ConstraintNotNull ) ) )
{
valid = valid && !value.isNull();
if ( value.isNull() )
bool exempt = false;
if ( fields.fieldOrigin( attributeIndex ) == QgsFields::OriginProvider
&& constraints.constraintOrigin( QgsFieldConstraints::ConstraintNotNull ) == QgsFieldConstraints::ConstraintOriginProvider )
{
errors << QObject::tr( "value is NULL" );
int providerIdx = fields.fieldOriginIndex( attributeIndex );
exempt = layer->dataProvider()->skipConstraintCheck( providerIdx, QgsFieldConstraints::ConstraintNotNull, value );
}
if ( !exempt )
{
valid = valid && !value.isNull();
if ( value.isNull() )
{
errors << QObject::tr( "value is NULL" );
}
}
}
@ -191,12 +203,23 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
&& ( strength == QgsFieldConstraints::ConstraintStrengthNotSet || strength == constraints.constraintStrength( QgsFieldConstraints::ConstraintUnique ) )
&& ( origin == QgsFieldConstraints::ConstraintOriginNotSet || origin == constraints.constraintOrigin( QgsFieldConstraints::ConstraintUnique ) ) )
{
bool alreadyExists = QgsVectorLayerUtils::valueExists( layer, attributeIndex, value, QgsFeatureIds() << feature.id() );
valid = valid && !alreadyExists;
if ( alreadyExists )
bool exempt = false;
if ( fields.fieldOrigin( attributeIndex ) == QgsFields::OriginProvider
&& constraints.constraintOrigin( QgsFieldConstraints::ConstraintNotNull ) == QgsFieldConstraints::ConstraintOriginProvider )
{
errors << QObject::tr( "value is not unique" );
int providerIdx = fields.fieldOriginIndex( attributeIndex );
exempt = layer->dataProvider()->skipConstraintCheck( providerIdx, QgsFieldConstraints::ConstraintUnique, value );
}
if ( !exempt )
{
bool alreadyExists = QgsVectorLayerUtils::valueExists( layer, attributeIndex, value, QgsFeatureIds() << feature.id() );
valid = valid && !alreadyExists;
if ( alreadyExists )
{
errors << QObject::tr( "value is not unique" );
}
}
}

View File

@ -1806,6 +1806,20 @@ QVariant QgsPostgresProvider::defaultValue( int fieldId ) const
return QVariant();
}
bool QgsPostgresProvider::skipConstraintCheck( int fieldIndex, QgsFieldConstraints::Constraint, const QVariant& value ) const
{
if ( providerProperty( EvaluateDefaultValues, false ).toBool() )
{
return mDefaultValues.contains( fieldIndex );
}
else
{
// stricter check - if we are evaluating default values only on commit then we can only bypass the check
// if the attribute values matches the original default clause
return mDefaultValues.contains( fieldIndex ) && mDefaultValues.value( fieldIndex ) == value.toString();
}
}
QString QgsPostgresProvider::paramValue( const QString& fieldValue, const QString &defaultValue ) const
{
if ( fieldValue.isNull() )

View File

@ -164,6 +164,7 @@ class QgsPostgresProvider : public QgsVectorDataProvider
QgsAttributeList pkAttributeIndexes() const override { return mPrimaryKeyAttrs; }
QString defaultValueClause( int fieldId ) const override;
QVariant defaultValue( int fieldId ) const override;
bool skipConstraintCheck( int fieldIndex, QgsFieldConstraints::Constraint constraint, const QVariant& value = QVariant() ) const override;
/** Adds a list of features
@return true in case of success and false in case of failure*/

View File

@ -520,14 +520,28 @@ class TestPyQgsPostgresProvider(unittest.TestCase, ProviderTestCase):
def testVectorLayerUtilsUniqueWithProviderDefault(self):
vl = QgsVectorLayer('%s table="qgis_test"."someData" sql=' % (self.dbconn), "someData", "postgres")
default_clause = 'nextval(\'qgis_test."someData_pk_seq"\'::regclass)'
vl.dataProvider().setProviderProperty(QgsDataProvider.EvaluateDefaultValues, False)
self.assertEqual(vl.dataProvider().defaultValueClause(0), default_clause)
self.assertTrue(QgsVectorLayerUtils.valueExists(vl, 0, 4))
vl.startEditing()
f = QgsFeature(vl.fields())
f.setAttribute(0, default_clause)
self.assertTrue(vl.addFeatures([f]))
self.assertFalse(QgsVectorLayerUtils.valueExists(vl, 0, default_clause))
self.assertTrue(vl.addFeatures([f]))
# the default value clause should exist...
self.assertTrue(QgsVectorLayerUtils.valueExists(vl, 0, default_clause))
# but it should not prevent the attribute being validated
self.assertTrue(QgsVectorLayerUtils.validateAttribute(vl, f, 0))
vl.rollBack()
def testSkipConstraintCheck(self):
vl = QgsVectorLayer('%s table="qgis_test"."someData" sql=' % (self.dbconn), "someData", "postgres")
default_clause = 'nextval(\'qgis_test."someData_pk_seq"\'::regclass)'
vl.dataProvider().setProviderProperty(QgsDataProvider.EvaluateDefaultValues, False)
self.assertTrue(vl.dataProvider().skipConstraintCheck(0, QgsFieldConstraints.ConstraintUnique, default_clause))
self.assertFalse(vl.dataProvider().skipConstraintCheck(0, QgsFieldConstraints.ConstraintUnique, 59))
def testVectorLayerUtilsCreateFeatureWithProviderDefault(self):
vl = QgsVectorLayer('%s table="qgis_test"."someData" sql=' % (self.dbconn), "someData", "postgres")