Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache unique values when creating features #9203

Merged
merged 20 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 49 additions & 0 deletions python/core/auto_generated/qgsvectorlayerutils.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,40 @@ Returns the duplicated features in the given layer

};

class QgsFeatureData
{
%Docstring
Encapsulate geometry and attributes for new features, to be passed to createFeatures

.. seealso:: :py:func:`createFeatures`

.. versionadded:: 3.6
%End

%TypeHeaderCode
#include "qgsvectorlayerutils.h"
%End
public:

QgsFeatureData( const QgsGeometry &geometry = QgsGeometry(), const QgsAttributeMap &attributes = QgsAttributeMap() );
%Docstring
Constructs a new QgsFeatureData with given ``geometry`` and ``attributes``
%End

QgsGeometry geometry() const;
%Docstring
Returns geometry
%End

QgsAttributeMap attributes() const;
%Docstring
Returns attributes
%End

};

typedef QList<QgsVectorLayerUtils::QgsFeatureData> QgsFeaturesDataList;

static QgsFeatureIterator getValuesIterator( const QgsVectorLayer *layer, const QString &fieldOrExpression, bool &ok, bool selectedOnly );
%Docstring
Create a feature iterator for a specified field name or expression.
Expand Down Expand Up @@ -143,6 +177,21 @@ Creates a new feature ready for insertion into a layer. Default values and const
passed for the new feature to copy as many attribute values as possible from the map,
assuming that they respect the layer's constraints. Note that the created feature is not
automatically inserted into the layer.

.. seealso:: :py:func:`createFeatures`
%End

static QgsFeatureList createFeatures( const QgsVectorLayer *layer,
const QgsFeaturesDataList &featuresData,
QgsExpressionContext *context = 0 );
%Docstring
Creates a set of new features ready for insertion into a layer. Default values and constraints
(e.g., unique constraints) will automatically be handled. Note that the created features are not
automatically inserted into the layer.

.. seealso:: :py:func:`createFeature`

.. versionadded:: 3.6
%End

static QgsFeature duplicateFeature( QgsVectorLayer *layer, const QgsFeature &feature, QgsProject *project, int depth, QgsDuplicateFeatureContext &duplicateFeatureContext /Out/ );
Expand Down
10 changes: 6 additions & 4 deletions src/app/qgisapp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8996,11 +8996,12 @@ void QgisApp::pasteFromClipboard( QgsMapLayer *destinationLayer )
QgsExpressionContext context = pasteVectorLayer->createExpressionContext();

QgsFeatureList compatibleFeatures( QgsVectorLayerUtils::makeFeaturesCompatible( features, pasteVectorLayer ) );
QgsFeatureList newFeatures;
QgsVectorLayerUtils::QgsFeaturesDataList newFeaturesDataList;
newFeaturesDataList.reserve( compatibleFeatures.size() );

// Count collapsed geometries
int invalidGeometriesCount = 0;

newFeatures.reserve( compatibleFeatures.size() );
for ( const auto &feature : qgis::as_const( compatibleFeatures ) )
{

Expand All @@ -9022,8 +9023,10 @@ void QgisApp::pasteFromClipboard( QgsMapLayer *destinationLayer )
}
// now create new feature using pasted feature as a template. This automatically handles default
// values and field constraints
newFeatures << QgsVectorLayerUtils::createFeature( pasteVectorLayer, geom, attrMap, &context );
newFeaturesDataList << QgsVectorLayerUtils::QgsFeatureData( geom, attrMap );
}

QgsFeatureList newFeatures {QgsVectorLayerUtils::createFeatures( pasteVectorLayer, newFeaturesDataList, &context )};
pasteVectorLayer->addFeatures( newFeatures );
QgsFeatureIds newIds;
newIds.reserve( newFeatures.size() );
Expand All @@ -9032,7 +9035,6 @@ void QgisApp::pasteFromClipboard( QgsMapLayer *destinationLayer )
newIds << f.id();
}


pasteVectorLayer->selectByIds( newIds );
pasteVectorLayer->endEditCommand();
pasteVectorLayer->updateExtents();
Expand Down
174 changes: 106 additions & 68 deletions src/core/qgsvectorlayerutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,18 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer *layer, const

QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, const QgsGeometry &geometry,
const QgsAttributeMap &attributes, QgsExpressionContext *context )
{
QgsFeatureList features { createFeatures( layer, QgsFeaturesDataList() << QgsFeatureData( geometry, attributes ), context ) };
return features.isEmpty() ? QgsFeature() : features.first();
}

QgsFeatureList QgsVectorLayerUtils::createFeatures( const QgsVectorLayer *layer, const QgsFeaturesDataList &featuresData, QgsExpressionContext *context )
{
if ( !layer )
{
return QgsFeature();
}
return QgsFeatureList();

QgsFeatureList result;
result.reserve( featuresData.length() );

QgsExpressionContext *evalContext = context;
std::unique_ptr< QgsExpressionContext > tempContext;
Expand All @@ -375,88 +382,104 @@ QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, cons

QgsFields fields = layer->fields();

QgsFeature newFeature( fields );
newFeature.setValid( true );
newFeature.setGeometry( geometry );
// Cache unique values
QMap<int, QSet<QVariant>> uniqueValueCaches;

// initialize attributes
newFeature.initAttributes( fields.count() );
for ( int idx = 0; idx < fields.count(); ++idx )
for ( const auto &fd : qgis::as_const( featuresData ) )
{
QVariant v;
bool checkUnique = true;

// in order of priority:
// 1. passed attribute value and if field does not have a unique constraint like primary key
if ( attributes.contains( idx ) )
{
v = attributes.value( idx );
}
QgsFeature newFeature( fields );
newFeature.setValid( true );
newFeature.setGeometry( fd.geometry() );

// 2. client side default expression
// note - deliberately not using else if!
if ( ( !v.isValid() || ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
&& layer->defaultValueDefinition( idx ).isValid() )
// initialize attributes
newFeature.initAttributes( fields.count() );
for ( int idx = 0; idx < fields.count(); ++idx )
{
// client side default expression set - takes precedence over all. Why? Well, this is the only default
// which QGIS users have control over, so we assume that they're deliberately overriding any
// provider defaults for some good reason and we should respect that
v = layer->defaultValue( idx, newFeature, evalContext );
}
QVariant v;
bool checkUnique = true;
const bool hasUniqueConstraint { static_cast<bool>( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique ) };

// 3. provider side default value clause
// note - not an else if deliberately. Users may return null from a default value expression to fallback to provider defaults
if ( ( !v.isValid() || ( fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
&& fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
{
int providerIndex = fields.fieldOriginIndex( idx );
QString providerDefault = layer->dataProvider()->defaultValueClause( providerIndex );
if ( !providerDefault.isEmpty() )
// in order of priority:
// 1. passed attribute value and if field does not have a unique constraint like primary key
if ( fd.attributes().contains( idx ) )
{
v = providerDefault;
checkUnique = false;
v = fd.attributes().value( idx );
elpaso marked this conversation as resolved.
Show resolved Hide resolved
}
}

// 4. provider side default literal
// note - deliberately not using else if!
if ( ( !v.isValid() || ( checkUnique && fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique && QgsVectorLayerUtils::valueExists( layer, idx, v ) ) )
&& fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
{
int providerIndex = fields.fieldOriginIndex( idx );
v = layer->dataProvider()->defaultValue( providerIndex );
if ( v.isValid() )
// Cache unique values
if ( hasUniqueConstraint && ! uniqueValueCaches.contains( idx ) )
uniqueValueCaches[ idx ] = layer->uniqueValues( idx );

// 2. client side default expression
// note - deliberately not using else if!
if ( ( !v.isValid() || ( hasUniqueConstraint
&& uniqueValueCaches[ idx ].contains( v ) ) )
&& layer->defaultValueDefinition( idx ).isValid() )
{
//trust that the provider default has been sensibly set not to violate any constraints
checkUnique = false;
// client side default expression set - takes precedence over all. Why? Well, this is the only default
// which QGIS users have control over, so we assume that they're deliberately overriding any
// provider defaults for some good reason and we should respect that
v = layer->defaultValue( idx, newFeature, evalContext );
}
}

// 5. passed attribute value
// note - deliberately not using else if!
if ( !v.isValid() && attributes.contains( idx ) )
{
v = attributes.value( idx );
}
// 3. provider side default value clause
// note - not an else if deliberately. Users may return null from a default value expression to fallback to provider defaults
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want that? NULL to fallback to provider defaults... different discussion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the check below is for isValid() and the comment here says NULL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, different discussion (that we need to do sooner or later), I called this monster method for a reason, I don't entirely get the logic here since there are a lot of different permutations of use cases.

if ( ( !v.isValid() || ( hasUniqueConstraint
&& uniqueValueCaches[ idx ].contains( v ) ) )
&& fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
{
int providerIndex = fields.fieldOriginIndex( idx );
QString providerDefault = layer->dataProvider()->defaultValueClause( providerIndex );
if ( !providerDefault.isEmpty() )
{
v = providerDefault;
checkUnique = false;
}
}

// last of all... check that unique constraints are respected
// we can't handle not null or expression constraints here, since there's no way to pick a sensible
// value if the constraint is violated
if ( checkUnique && fields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique )
{
if ( QgsVectorLayerUtils::valueExists( layer, idx, v ) )
// 4. provider side default literal
// note - deliberately not using else if!
if ( ( !v.isValid() || ( checkUnique && hasUniqueConstraint
&& uniqueValueCaches[ idx ].contains( v ) ) )
&& fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
{
// unique constraint violated
QVariant uniqueValue = QgsVectorLayerUtils::createUniqueValue( layer, idx, v );
if ( uniqueValue.isValid() )
v = uniqueValue;
int providerIndex = fields.fieldOriginIndex( idx );
v = layer->dataProvider()->defaultValue( providerIndex );
if ( v.isValid() )
{
//trust that the provider default has been sensibly set not to violate any constraints
checkUnique = false;
}
}
}

newFeature.setAttribute( idx, v );
}
// 5. passed attribute value
// note - deliberately not using else if!
if ( !v.isValid() && fd.attributes().contains( idx ) )
{
v = fd.attributes().value( idx );
}

return newFeature;
// last of all... check that unique constraints are respected
// we can't handle not null or expression constraints here, since there's no way to pick a sensible
// value if the constraint is violated
if ( checkUnique && hasUniqueConstraint )
{
if ( uniqueValueCaches[ idx ].contains( v ) )
{
// unique constraint violated
QVariant uniqueValue = QgsVectorLayerUtils::createUniqueValue( layer, idx, v );
if ( uniqueValue.isValid() )
v = uniqueValue;
}
}
if ( hasUniqueConstraint )
uniqueValueCaches[ idx ].insert( v );
newFeature.setAttribute( idx, v );
}
result.append( newFeature );
}
return result;
}

QgsFeature QgsVectorLayerUtils::duplicateFeature( QgsVectorLayer *layer, const QgsFeature &feature, QgsProject *project, int depth, QgsDuplicateFeatureContext &duplicateFeatureContext )
Expand Down Expand Up @@ -766,3 +789,18 @@ QMap<QgsVectorLayer *, QgsFeatureIds> QgsVectorLayerUtils::QgsDuplicateFeatureC
return mDuplicatedFeatures;
}
*/

QgsVectorLayerUtils::QgsFeatureData::QgsFeatureData( const QgsGeometry &geometry, const QgsAttributeMap &attributes ):
mGeometry( geometry ),
mAttributes( attributes )
{}

QgsGeometry QgsVectorLayerUtils::QgsFeatureData::geometry() const
{
return mGeometry;
}

QgsAttributeMap QgsVectorLayerUtils::QgsFeatureData::attributes() const
{
return mAttributes;
}
43 changes: 43 additions & 0 deletions src/core/qgsvectorlayerutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,37 @@ class CORE_EXPORT QgsVectorLayerUtils
void setDuplicatedFeatures( QgsVectorLayer *layer, const QgsFeatureIds &ids );
};

/**
* \ingroup core
* \class QgsFeatureData
* \brief Encapsulate geometry and attributes for new features, to be passed to createFeatures
* \see createFeatures()
* \since QGIS 3.6
*/
class CORE_EXPORT QgsFeatureData
{
public:

/**
* Constructs a new QgsFeatureData with given \a geometry and \a attributes
*/
QgsFeatureData( const QgsGeometry &geometry = QgsGeometry(), const QgsAttributeMap &attributes = QgsAttributeMap() );

//! Returns geometry
QgsGeometry geometry() const;

//! Returns attributes
QgsAttributeMap attributes() const;

private:
QgsGeometry mGeometry;
QgsAttributeMap mAttributes;
};

// SIP does not lile "using", use legacy typedef
elpaso marked this conversation as resolved.
Show resolved Hide resolved
//! Alias for list of QgsFeatureData
typedef QList<QgsVectorLayerUtils::QgsFeatureData> QgsFeaturesDataList;

/**
* Create a feature iterator for a specified field name or expression.
* \param layer vector layer to retrieve values from
Expand Down Expand Up @@ -139,12 +170,24 @@ class CORE_EXPORT QgsVectorLayerUtils
* passed for the new feature to copy as many attribute values as possible from the map,
* assuming that they respect the layer's constraints. Note that the created feature is not
* automatically inserted into the layer.
* \see createFeatures()
*/
static QgsFeature createFeature( const QgsVectorLayer *layer,
const QgsGeometry &geometry = QgsGeometry(),
const QgsAttributeMap &attributes = QgsAttributeMap(),
QgsExpressionContext *context = nullptr );

/**
* Creates a set of new features ready for insertion into a layer. Default values and constraints
* (e.g., unique constraints) will automatically be handled. Note that the created features are not
* automatically inserted into the layer.
* \see createFeature()
* \since QGIS 3.6
*/
static QgsFeatureList createFeatures( const QgsVectorLayer *layer,
const QgsFeaturesDataList &featuresData,
QgsExpressionContext *context = nullptr );

/**
* Duplicates a feature and it's children (one level deep). It calls CreateFeature, so
* default values and constraints (e.g., unique constraints) will automatically be handled.
Expand Down