Skip to content

Commit

Permalink
GPKG and spatialite AUTOINCREMENT: get next value from sequence
Browse files Browse the repository at this point in the history 
for PK default value, fixes #37222

Also, fix dangling transactions for spatialite.
  • Loading branch information
@elpaso @nyalldawson
elpaso authored and nyalldawson committed Jun 19, 2020
1 parent 2e6fdbe commit 4b36262
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 33 deletions.
1 change: 1 addition & 0 deletions python/core/auto_generated/qgssqliteutils.sip.in
View file
Expand Up @@ -54,6 +54,7 @@ Returns a string list of SQLite (and spatialite) system tables
%End



};

/************************************************************************
Expand Down
11 changes: 0 additions & 11 deletions src/app/qgsfeatureaction.cpp
View file
Expand Up @@ -249,17 +249,6 @@ bool QgsFeatureAction::addFeature( const QgsAttributeMap &defaultAttributes, boo
}
else
{
// If the layer is inside a transaction group we need to add
// the feature first to get the provider-evaluated defaults
const bool inTransaction { mLayer->dataProvider() &&
mLayer->dataProvider()->transaction() };
if ( inTransaction )
{
if ( mLayer->addFeature( *mFeature ) )
{
mLayer->deleteFeature( mFeature->id() );
}
}

QgsAttributeDialog *dialog = newDialog( false );
// delete the dialog when it is closed
Expand Down
61 changes: 60 additions & 1 deletion src/core/providers/ogr/qgsogrprovider.cpp
View file
Expand Up @@ -1452,13 +1452,72 @@ QVariant QgsOgrProvider::defaultValue( int fieldId ) const
else if ( defaultVal == QStringLiteral( "CURRENT_TIME" ) )
resultVar = QTime::currentTime();

// Get next sequence value for sqlite in case we are inside a transaction
if ( mOgrOrigLayer &&
mTransaction &&
mDefaultValues.value( fieldId, QString() ) == tr( "Autogenerate" ) &&
providerProperty( EvaluateDefaultValues, false ).toBool() &&
( mGDALDriverName == QLatin1String( "GPKG" ) ||
mGDALDriverName == QLatin1String( "SQLite" ) ) &&
mFirstFieldIsFid &&
fieldId == 0 )
{
QgsOgrLayerUniquePtr resultLayer = mOgrOrigLayer->ExecuteSQL( QByteArray( "SELECT seq FROM sqlite_sequence WHERE name = " ) + QgsSqliteUtils::quotedValue( mOgrOrigLayer->name() ).toUtf8() );
if ( resultLayer )
{
gdal::ogr_feature_unique_ptr f;
if ( f.reset( resultLayer->GetNextFeature() ), f )
{
bool ok { true };
const QVariant res = QgsOgrUtils::getOgrFeatureAttribute( f.get(),
fields().at( 0 ),
0, textEncoding(), &ok );
if ( ok )
{
long long nextVal { res.toLongLong( &ok ) };
if ( ok )
{
// Increment
resultVar = ++nextVal;
mOgrOrigLayer->ExecuteSQLNoReturn( QByteArray( "UPDATE sqlite_sequence SET seq = seq + 1 WHERE name = " ) + QgsSqliteUtils::quotedValue( mOgrOrigLayer->name() ).toUtf8() );
}
}

if ( ! ok )
{
QgsMessageLog::logMessage( tr( "Error retrieving next sequence value for %1" ).arg( QString( mOgrOrigLayer->name() ) ), tr( "OGR" ) );
}
}
else // no sequence!
{
resultVar = 1;
mOgrOrigLayer->ExecuteSQLNoReturn( QByteArray( "INSERT INTO sqlite_sequence (name, seq) VALUES( " +
QgsSqliteUtils::quotedValue( mOgrOrigLayer->name() ).toUtf8() ) + ", 1)" );
}
}
else
{
QgsMessageLog::logMessage( tr( "Error retrieving default value for %1" ).arg( mLayerName ), tr( "OGR" ) );
}
}

( void )mAttributeFields.at( fieldId ).convertCompatible( resultVar );
return resultVar;
}

QString QgsOgrProvider::defaultValueClause( int fieldIndex ) const
{
return mDefaultValues.value( fieldIndex, QString() );
// Return empty clause to force defaultValue calls for sqlite in case we are inside a transaction
if ( mTransaction &&
mDefaultValues.value( fieldIndex, QString() ) == tr( "Autogenerate" ) &&
providerProperty( EvaluateDefaultValues, false ).toBool() &&
( mGDALDriverName == QLatin1String( "GPKG" ) ||
mGDALDriverName == QLatin1String( "SQLite" ) ) &&
mFirstFieldIsFid &&
fieldIndex == 0 )
return QString();
else
return mDefaultValues.value( fieldIndex, QString() );
}

bool QgsOgrProvider::skipConstraintCheck( int fieldIndex, QgsFieldConstraints::Constraint constraint, const QVariant &value ) const
Expand Down
43 changes: 43 additions & 0 deletions src/core/qgssqliteutils.cpp
View file
Expand Up @@ -203,6 +203,49 @@ QSet<QString> QgsSqliteUtils::uniqueFields( sqlite3 *connection, const QString &
return uniqueFieldsResults;
}

long long QgsSqliteUtils::nextSequenceValue( sqlite3 *connection, const QString &tableName, QString errorMessage )
{
long long result { -1 };
sqlite3_database_unique_ptr dsPtr;
dsPtr.reset( connection );
const QString quotedTableName { QgsSqliteUtils::quotedValue( tableName ) };

int resultCode;
sqlite3_statement_unique_ptr stmt { dsPtr.prepare( QStringLiteral( "SELECT seq FROM sqlite_sequence WHERE name = %1" )
.arg( quotedTableName ), resultCode )};
if ( resultCode == SQLITE_OK )
{
stmt.step();
result = sqlite3_column_int64( stmt.get(), 0 );
// Try to create the sequence in case this is an empty layer
if ( sqlite3_column_count( stmt.get() ) == 0 )
{
dsPtr.exec( QStringLiteral( "INSERT INTO sqlite_sequence (name, seq) VALUES (%1, 1)" ).arg( quotedTableName ), errorMessage );
if ( errorMessage.isEmpty() )
{
result = 1;
}
else
{
errorMessage = QObject::tr( "Error retrieving default value for %1" ).arg( tableName );
}
}
else // increment
{
if ( dsPtr.exec( QStringLiteral( "UPDATE sqlite_sequence SET seq = %1 WHERE name = %2" )
.arg( QString::number( ++result ) )
.arg( quotedTableName ), errorMessage ) != SQLITE_OK )
{
errorMessage = QObject::tr( "Error retrieving default value for %1" ).arg( tableName );
result = -1;
}
}
}

dsPtr.release();
return result;
}

QString QgsSqliteUtils::quotedString( const QString &value )
{
if ( value.isNull() )
Expand Down
11 changes: 11 additions & 0 deletions src/core/qgssqliteutils.h
View file
Expand Up @@ -213,6 +213,17 @@ class CORE_EXPORT QgsSqliteUtils
*/
static QSet<QString> uniqueFields( sqlite3 *connection, const QString &tableName, QString &errorMessage ) SIP_SKIP;

/**
* Increments and returns an SQLITE sequence of the table "sqlite_sequence"
* for \a tableName and returns it value, \a errorMessage is filled with the
* error message in case of errors.
*
* \returns the next sequence value or -1 case of errors
* \since QGIS 3.14
* \note not available in Python bindings
*/
static long long nextSequenceValue( sqlite3 *connection, const QString &tableName, QString errorMessage ) SIP_SKIP;

};

#endif // QGSSQLITEUTILS_H
3 changes: 2 additions & 1 deletion src/core/qgsvectorlayerutils.cpp
View file
Expand Up @@ -573,7 +573,8 @@ QgsFeatureList QgsVectorLayerUtils::createFeatures( const QgsVectorLayer *layer,

// 4. provider side default literal
// note - deliberately not using else if!
if ( ( v.isNull() || ( checkUnique && hasUniqueConstraint
if ( ( v.isNull() || ( checkUnique
&& hasUniqueConstraint
&& checkUniqueValue( idx, v ) ) )
&& fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
{
Expand Down
2 changes: 1 addition & 1 deletion src/providers/postgres/qgspostgresprovider.cpp
View file
Expand Up @@ -2084,7 +2084,7 @@ QString QgsPostgresProvider::defaultValueClause( int fieldId ) const
// Here, we return the expression used to generate the field value, so the
// user can see what is happening when inserting a new feature.
// On inserting a new feature or updating a generated field, this is
// ommited from the generated queries.
// omitted from the generated queries.
// See https://www.postgresql.org/docs/12/ddl-generated-columns.html
if ( !genVal.isEmpty() )
{
Expand Down
39 changes: 35 additions & 4 deletions src/providers/spatialite/qgsspatialiteprovider.cpp
View file
Expand Up @@ -642,6 +642,14 @@ QgsSpatiaLiteProvider::QgsSpatiaLiteProvider( QString const &uri, const Provider

QgsSpatiaLiteProvider::~QgsSpatiaLiteProvider()
{
if ( mTransaction )
{
QString errorMessage;
if ( ! mTransaction->rollback( errorMessage ) )
{
QgsMessageLog::logMessage( tr( "Error closing transaction for %1" ).arg( mTableName ), tr( "SpatiaLite" ) );
}
}
closeDb();
invalidateConnections( mSqlitePath );
}
Expand Down Expand Up @@ -1053,7 +1061,6 @@ void QgsSpatiaLiteProvider::insertDefaultValue( int fieldIndex, QString defaultV
}
}


QVariant QgsSpatiaLiteProvider::defaultValue( int fieldId ) const
{
// TODO: backend-side evaluation
Expand All @@ -1079,6 +1086,24 @@ QVariant QgsSpatiaLiteProvider::defaultValue( int fieldId ) const
resultVar = defaultVal;
}

if ( mTransaction &&
mAttributeFields.at( fieldId ).name() == mPrimaryKey &&
mPrimaryKeyAutoIncrement &&
mDefaultValues.value( fieldId, QString() ) == tr( "Autogenerate" ) &&
providerProperty( EvaluateDefaultValues, false ).toBool() )
{
QString errorMessage;
QVariant nextVal { QgsSqliteUtils::nextSequenceValue( sqliteHandle(), mTableName, errorMessage ) };
if ( errorMessage.isEmpty() && nextVal != -1 )
{
resultVar = nextVal;
}
else
{
QgsMessageLog::logMessage( errorMessage, tr( "SpatiaLite" ) );
}
}

( void )mAttributeFields.at( fieldId ).convertCompatible( resultVar );
return resultVar;
}
Expand All @@ -1092,7 +1117,15 @@ QString QgsSpatiaLiteProvider::defaultValueClause( int fieldIndex ) const

if ( mAttributeFields.at( fieldIndex ).name() == mPrimaryKey && mPrimaryKeyAutoIncrement )
{
return tr( "Autogenerate" );
if ( mTransaction &&
providerProperty( EvaluateDefaultValues, false ).toBool() )
{
return QString();
}
else
{
return tr( "Autogenerate" );
}
}
return mDefaultValueClause.value( fieldIndex, QString() );
}
Expand Down Expand Up @@ -1265,7 +1298,6 @@ void QgsSpatiaLiteProvider::loadFields()
updatePrimaryKeyCapabilities();
}


void QgsSpatiaLiteProvider::determineViewPrimaryKey()
{
QString sql = QString( "SELECT view_rowid"
Expand Down Expand Up @@ -1333,7 +1365,6 @@ QStringList QgsSpatiaLiteProvider::tablePrimaryKeys( const QString &tableName )
return result;
}


bool QgsSpatiaLiteProvider::hasTriggers()
{
int ret;
Expand Down
24 changes: 12 additions & 12 deletions src/ui/qgsprojectpropertiesbase.ui
View file
Expand Up @@ -245,7 +245,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 @@ -274,8 +274,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>590</width>
<height>828</height>
<width>671</width>
<height>847</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout">
Expand Down Expand Up @@ -897,8 +897,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>603</width>
<height>161</height>
<width>685</width>
<height>680</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_7">
Expand Down Expand Up @@ -972,8 +972,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>290</width>
<height>543</height>
<width>685</width>
<height>680</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_12">
Expand Down Expand Up @@ -1392,7 +1392,7 @@
<item row="0" column="0" colspan="2">
<widget class="QCheckBox" name="mAutoTransaction">
<property name="toolTip">
<string>When enabled, layers from the same database connection will be put into a transaction group. Their edit state will be synchronized and changes to these layers will be sent to the provider immediately. Only supported on postgres provider.</string>
<string>When enabled, layers from the same database connection will be put into a transaction group. Their edit state will be synchronized and changes to these layers will be sent to the provider immediately. Only supported for postgres, GPKG, spatialite and oracle.</string>
</property>
<property name="text">
<string>Automatically create transaction groups where possible</string>
Expand All @@ -1402,7 +1402,7 @@
<item row="1" column="0" colspan="2">
<widget class="QCheckBox" name="mEvaluateDefaultValues">
<property name="toolTip">
<string>When enabled, default values will be evaluated as early as possible. This will fill default values in the add feature form already and not only create them on commit. Only supported for postgres provider.</string>
<string>When enabled, default values will be evaluated as early as possible. This will fill default values in the add feature form already and not only create them on commit. Only supported for postgres, GPKG, spatialite and oracle.</string>
</property>
<property name="text">
<string>Evaluate default values on provider side</string>
Expand Down Expand Up @@ -1548,8 +1548,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">
Expand Down Expand Up @@ -1611,7 +1611,7 @@
<x>0</x>
<y>0</y>
<width>671</width>
<height>3090</height>
<height>3139</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_13">
Expand Down

0 comments on commit 4b36262

Please sign in to comment.