Skip to content

Commit 4b36262

Browse files
elpasonyalldawson
authored andcommittedJun 19, 2020
GPKG and spatialite AUTOINCREMENT: get next value from sequence
for PK default value, fixes #37222 Also, fix dangling transactions for spatialite.
1 parent 2e6fdbe commit 4b36262

File tree

10 files changed

+202
-33
lines changed

10 files changed

+202
-33
lines changed
 

‎python/core/auto_generated/qgssqliteutils.sip.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ Returns a string list of SQLite (and spatialite) system tables
5454
%End
5555

5656

57+
5758
};
5859

5960
/************************************************************************

‎src/app/qgsfeatureaction.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -249,17 +249,6 @@ bool QgsFeatureAction::addFeature( const QgsAttributeMap &defaultAttributes, boo
249249
}
250250
else
251251
{
252-
// If the layer is inside a transaction group we need to add
253-
// the feature first to get the provider-evaluated defaults
254-
const bool inTransaction { mLayer->dataProvider() &&
255-
mLayer->dataProvider()->transaction() };
256-
if ( inTransaction )
257-
{
258-
if ( mLayer->addFeature( *mFeature ) )
259-
{
260-
mLayer->deleteFeature( mFeature->id() );
261-
}
262-
}
263252

264253
QgsAttributeDialog *dialog = newDialog( false );
265254
// delete the dialog when it is closed

‎src/core/providers/ogr/qgsogrprovider.cpp

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1452,13 +1452,72 @@ QVariant QgsOgrProvider::defaultValue( int fieldId ) const
14521452
else if ( defaultVal == QStringLiteral( "CURRENT_TIME" ) )
14531453
resultVar = QTime::currentTime();
14541454

1455+
// Get next sequence value for sqlite in case we are inside a transaction
1456+
if ( mOgrOrigLayer &&
1457+
mTransaction &&
1458+
mDefaultValues.value( fieldId, QString() ) == tr( "Autogenerate" ) &&
1459+
providerProperty( EvaluateDefaultValues, false ).toBool() &&
1460+
( mGDALDriverName == QLatin1String( "GPKG" ) ||
1461+
mGDALDriverName == QLatin1String( "SQLite" ) ) &&
1462+
mFirstFieldIsFid &&
1463+
fieldId == 0 )
1464+
{
1465+
QgsOgrLayerUniquePtr resultLayer = mOgrOrigLayer->ExecuteSQL( QByteArray( "SELECT seq FROM sqlite_sequence WHERE name = " ) + QgsSqliteUtils::quotedValue( mOgrOrigLayer->name() ).toUtf8() );
1466+
if ( resultLayer )
1467+
{
1468+
gdal::ogr_feature_unique_ptr f;
1469+
if ( f.reset( resultLayer->GetNextFeature() ), f )
1470+
{
1471+
bool ok { true };
1472+
const QVariant res = QgsOgrUtils::getOgrFeatureAttribute( f.get(),
1473+
fields().at( 0 ),
1474+
0, textEncoding(), &ok );
1475+
if ( ok )
1476+
{
1477+
long long nextVal { res.toLongLong( &ok ) };
1478+
if ( ok )
1479+
{
1480+
// Increment
1481+
resultVar = ++nextVal;
1482+
mOgrOrigLayer->ExecuteSQLNoReturn( QByteArray( "UPDATE sqlite_sequence SET seq = seq + 1 WHERE name = " ) + QgsSqliteUtils::quotedValue( mOgrOrigLayer->name() ).toUtf8() );
1483+
}
1484+
}
1485+
1486+
if ( ! ok )
1487+
{
1488+
QgsMessageLog::logMessage( tr( "Error retrieving next sequence value for %1" ).arg( QString( mOgrOrigLayer->name() ) ), tr( "OGR" ) );
1489+
}
1490+
}
1491+
else // no sequence!
1492+
{
1493+
resultVar = 1;
1494+
mOgrOrigLayer->ExecuteSQLNoReturn( QByteArray( "INSERT INTO sqlite_sequence (name, seq) VALUES( " +
1495+
QgsSqliteUtils::quotedValue( mOgrOrigLayer->name() ).toUtf8() ) + ", 1)" );
1496+
}
1497+
}
1498+
else
1499+
{
1500+
QgsMessageLog::logMessage( tr( "Error retrieving default value for %1" ).arg( mLayerName ), tr( "OGR" ) );
1501+
}
1502+
}
1503+
14551504
( void )mAttributeFields.at( fieldId ).convertCompatible( resultVar );
14561505
return resultVar;
14571506
}
14581507

14591508
QString QgsOgrProvider::defaultValueClause( int fieldIndex ) const
14601509
{
1461-
return mDefaultValues.value( fieldIndex, QString() );
1510+
// Return empty clause to force defaultValue calls for sqlite in case we are inside a transaction
1511+
if ( mTransaction &&
1512+
mDefaultValues.value( fieldIndex, QString() ) == tr( "Autogenerate" ) &&
1513+
providerProperty( EvaluateDefaultValues, false ).toBool() &&
1514+
( mGDALDriverName == QLatin1String( "GPKG" ) ||
1515+
mGDALDriverName == QLatin1String( "SQLite" ) ) &&
1516+
mFirstFieldIsFid &&
1517+
fieldIndex == 0 )
1518+
return QString();
1519+
else
1520+
return mDefaultValues.value( fieldIndex, QString() );
14621521
}
14631522

14641523
bool QgsOgrProvider::skipConstraintCheck( int fieldIndex, QgsFieldConstraints::Constraint constraint, const QVariant &value ) const

‎src/core/qgssqliteutils.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,49 @@ QSet<QString> QgsSqliteUtils::uniqueFields( sqlite3 *connection, const QString &
203203
return uniqueFieldsResults;
204204
}
205205

206+
long long QgsSqliteUtils::nextSequenceValue( sqlite3 *connection, const QString &tableName, QString errorMessage )
207+
{
208+
long long result { -1 };
209+
sqlite3_database_unique_ptr dsPtr;
210+
dsPtr.reset( connection );
211+
const QString quotedTableName { QgsSqliteUtils::quotedValue( tableName ) };
212+
213+
int resultCode;
214+
sqlite3_statement_unique_ptr stmt { dsPtr.prepare( QStringLiteral( "SELECT seq FROM sqlite_sequence WHERE name = %1" )
215+
.arg( quotedTableName ), resultCode )};
216+
if ( resultCode == SQLITE_OK )
217+
{
218+
stmt.step();
219+
result = sqlite3_column_int64( stmt.get(), 0 );
220+
// Try to create the sequence in case this is an empty layer
221+
if ( sqlite3_column_count( stmt.get() ) == 0 )
222+
{
223+
dsPtr.exec( QStringLiteral( "INSERT INTO sqlite_sequence (name, seq) VALUES (%1, 1)" ).arg( quotedTableName ), errorMessage );
224+
if ( errorMessage.isEmpty() )
225+
{
226+
result = 1;
227+
}
228+
else
229+
{
230+
errorMessage = QObject::tr( "Error retrieving default value for %1" ).arg( tableName );
231+
}
232+
}
233+
else // increment
234+
{
235+
if ( dsPtr.exec( QStringLiteral( "UPDATE sqlite_sequence SET seq = %1 WHERE name = %2" )
236+
.arg( QString::number( ++result ) )
237+
.arg( quotedTableName ), errorMessage ) != SQLITE_OK )
238+
{
239+
errorMessage = QObject::tr( "Error retrieving default value for %1" ).arg( tableName );
240+
result = -1;
241+
}
242+
}
243+
}
244+
245+
dsPtr.release();
246+
return result;
247+
}
248+
206249
QString QgsSqliteUtils::quotedString( const QString &value )
207250
{
208251
if ( value.isNull() )

‎src/core/qgssqliteutils.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,17 @@ class CORE_EXPORT QgsSqliteUtils
213213
*/
214214
static QSet<QString> uniqueFields( sqlite3 *connection, const QString &tableName, QString &errorMessage ) SIP_SKIP;
215215

216+
/**
217+
* Increments and returns an SQLITE sequence of the table "sqlite_sequence"
218+
* for \a tableName and returns it value, \a errorMessage is filled with the
219+
* error message in case of errors.
220+
*
221+
* \returns the next sequence value or -1 case of errors
222+
* \since QGIS 3.14
223+
* \note not available in Python bindings
224+
*/
225+
static long long nextSequenceValue( sqlite3 *connection, const QString &tableName, QString errorMessage ) SIP_SKIP;
226+
216227
};
217228

218229
#endif // QGSSQLITEUTILS_H

‎src/core/qgsvectorlayerutils.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,8 @@ QgsFeatureList QgsVectorLayerUtils::createFeatures( const QgsVectorLayer *layer,
573573

574574
// 4. provider side default literal
575575
// note - deliberately not using else if!
576-
if ( ( v.isNull() || ( checkUnique && hasUniqueConstraint
576+
if ( ( v.isNull() || ( checkUnique
577+
&& hasUniqueConstraint
577578
&& checkUniqueValue( idx, v ) ) )
578579
&& fields.fieldOrigin( idx ) == QgsFields::OriginProvider )
579580
{

‎src/providers/postgres/qgspostgresprovider.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2084,7 +2084,7 @@ QString QgsPostgresProvider::defaultValueClause( int fieldId ) const
20842084
// Here, we return the expression used to generate the field value, so the
20852085
// user can see what is happening when inserting a new feature.
20862086
// On inserting a new feature or updating a generated field, this is
2087-
// ommited from the generated queries.
2087+
// omitted from the generated queries.
20882088
// See https://www.postgresql.org/docs/12/ddl-generated-columns.html
20892089
if ( !genVal.isEmpty() )
20902090
{

‎src/providers/spatialite/qgsspatialiteprovider.cpp

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,14 @@ QgsSpatiaLiteProvider::QgsSpatiaLiteProvider( QString const &uri, const Provider
642642

643643
QgsSpatiaLiteProvider::~QgsSpatiaLiteProvider()
644644
{
645+
if ( mTransaction )
646+
{
647+
QString errorMessage;
648+
if ( ! mTransaction->rollback( errorMessage ) )
649+
{
650+
QgsMessageLog::logMessage( tr( "Error closing transaction for %1" ).arg( mTableName ), tr( "SpatiaLite" ) );
651+
}
652+
}
645653
closeDb();
646654
invalidateConnections( mSqlitePath );
647655
}
@@ -1053,7 +1061,6 @@ void QgsSpatiaLiteProvider::insertDefaultValue( int fieldIndex, QString defaultV
10531061
}
10541062
}
10551063

1056-
10571064
QVariant QgsSpatiaLiteProvider::defaultValue( int fieldId ) const
10581065
{
10591066
// TODO: backend-side evaluation
@@ -1079,6 +1086,24 @@ QVariant QgsSpatiaLiteProvider::defaultValue( int fieldId ) const
10791086
resultVar = defaultVal;
10801087
}
10811088

1089+
if ( mTransaction &&
1090+
mAttributeFields.at( fieldId ).name() == mPrimaryKey &&
1091+
mPrimaryKeyAutoIncrement &&
1092+
mDefaultValues.value( fieldId, QString() ) == tr( "Autogenerate" ) &&
1093+
providerProperty( EvaluateDefaultValues, false ).toBool() )
1094+
{
1095+
QString errorMessage;
1096+
QVariant nextVal { QgsSqliteUtils::nextSequenceValue( sqliteHandle(), mTableName, errorMessage ) };
1097+
if ( errorMessage.isEmpty() && nextVal != -1 )
1098+
{
1099+
resultVar = nextVal;
1100+
}
1101+
else
1102+
{
1103+
QgsMessageLog::logMessage( errorMessage, tr( "SpatiaLite" ) );
1104+
}
1105+
}
1106+
10821107
( void )mAttributeFields.at( fieldId ).convertCompatible( resultVar );
10831108
return resultVar;
10841109
}
@@ -1092,7 +1117,15 @@ QString QgsSpatiaLiteProvider::defaultValueClause( int fieldIndex ) const
10921117

10931118
if ( mAttributeFields.at( fieldIndex ).name() == mPrimaryKey && mPrimaryKeyAutoIncrement )
10941119
{
1095-
return tr( "Autogenerate" );
1120+
if ( mTransaction &&
1121+
providerProperty( EvaluateDefaultValues, false ).toBool() )
1122+
{
1123+
return QString();
1124+
}
1125+
else
1126+
{
1127+
return tr( "Autogenerate" );
1128+
}
10961129
}
10971130
return mDefaultValueClause.value( fieldIndex, QString() );
10981131
}
@@ -1265,7 +1298,6 @@ void QgsSpatiaLiteProvider::loadFields()
12651298
updatePrimaryKeyCapabilities();
12661299
}
12671300

1268-
12691301
void QgsSpatiaLiteProvider::determineViewPrimaryKey()
12701302
{
12711303
QString sql = QString( "SELECT view_rowid"
@@ -1333,7 +1365,6 @@ QStringList QgsSpatiaLiteProvider::tablePrimaryKeys( const QString &tableName )
13331365
return result;
13341366
}
13351367

1336-
13371368
bool QgsSpatiaLiteProvider::hasTriggers()
13381369
{
13391370
int ret;

‎src/ui/qgsprojectpropertiesbase.ui

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@
245245
</sizepolicy>
246246
</property>
247247
<property name="currentIndex">
248-
<number>8</number>
248+
<number>4</number>
249249
</property>
250250
<widget class="QWidget" name="mProjOptsGeneral">
251251
<layout class="QVBoxLayout" name="verticalLayout_6">
@@ -274,8 +274,8 @@
274274
<rect>
275275
<x>0</x>
276276
<y>0</y>
277-
<width>590</width>
278-
<height>828</height>
277+
<width>671</width>
278+
<height>847</height>
279279
</rect>
280280
</property>
281281
<layout class="QVBoxLayout" name="verticalLayout">
@@ -897,8 +897,8 @@
897897
<rect>
898898
<x>0</x>
899899
<y>0</y>
900-
<width>603</width>
901-
<height>161</height>
900+
<width>685</width>
901+
<height>680</height>
902902
</rect>
903903
</property>
904904
<layout class="QVBoxLayout" name="verticalLayout_7">
@@ -972,8 +972,8 @@
972972
<rect>
973973
<x>0</x>
974974
<y>0</y>
975-
<width>290</width>
976-
<height>543</height>
975+
<width>685</width>
976+
<height>680</height>
977977
</rect>
978978
</property>
979979
<layout class="QVBoxLayout" name="verticalLayout_12">
@@ -1392,7 +1392,7 @@
13921392
<item row="0" column="0" colspan="2">
13931393
<widget class="QCheckBox" name="mAutoTransaction">
13941394
<property name="toolTip">
1395-
<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>
1395+
<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>
13961396
</property>
13971397
<property name="text">
13981398
<string>Automatically create transaction groups where possible</string>
@@ -1402,7 +1402,7 @@
14021402
<item row="1" column="0" colspan="2">
14031403
<widget class="QCheckBox" name="mEvaluateDefaultValues">
14041404
<property name="toolTip">
1405-
<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>
1405+
<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>
14061406
</property>
14071407
<property name="text">
14081408
<string>Evaluate default values on provider side</string>
@@ -1548,8 +1548,8 @@
15481548
<rect>
15491549
<x>0</x>
15501550
<y>0</y>
1551-
<width>178</width>
1552-
<height>54</height>
1551+
<width>167</width>
1552+
<height>55</height>
15531553
</rect>
15541554
</property>
15551555
<layout class="QVBoxLayout" name="verticalLayout_17">
@@ -1611,7 +1611,7 @@
16111611
<x>0</x>
16121612
<y>0</y>
16131613
<width>671</width>
1614-
<height>3090</height>
1614+
<height>3139</height>
16151615
</rect>
16161616
</property>
16171617
<layout class="QVBoxLayout" name="verticalLayout_13">

‎tests/src/python/test_provider_spatialite.py

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ def setUpClass(cls):
153153
cur.execute(sql)
154154

155155
# simple table with a geometry column named 'Geometry'
156-
sql = "CREATE TABLE test_n (Id INTEGER NOT NULL PRIMARY KEY, name TEXT NOT NULL)"
156+
sql = "CREATE TABLE test_n (id INTEGER NOT NULL PRIMARY KEY, name TEXT NOT NULL)"
157157
cur.execute(sql)
158158
sql = "SELECT AddGeometryColumn('test_n', 'Geometry', 4326, 'POLYGON', 'XY')"
159159
cur.execute(sql)
@@ -165,7 +165,7 @@ def setUpClass(cls):
165165
cur.execute(sql)
166166

167167
# table with different array types, stored as JSON
168-
sql = "CREATE TABLE test_arrays (Id INTEGER NOT NULL PRIMARY KEY, strings JSONSTRINGLIST NOT NULL, ints JSONINTEGERLIST NOT NULL, reals JSONREALLIST NOT NULL)"
168+
sql = "CREATE TABLE test_arrays (id INTEGER NOT NULL PRIMARY KEY, strings JSONSTRINGLIST NOT NULL, ints JSONINTEGERLIST NOT NULL, reals JSONREALLIST NOT NULL)"
169169
cur.execute(sql)
170170
sql = "SELECT AddGeometryColumn('test_arrays', 'Geometry', 4326, 'POLYGON', 'XY')"
171171
cur.execute(sql)
@@ -174,7 +174,7 @@ def setUpClass(cls):
174174
cur.execute(sql)
175175

176176
# table with different array types, stored as JSON
177-
sql = "CREATE TABLE test_arrays_write (Id INTEGER NOT NULL PRIMARY KEY, array JSONARRAY NOT NULL, strings JSONSTRINGLIST NOT NULL, ints JSONINTEGERLIST NOT NULL, reals JSONREALLIST NOT NULL)"
177+
sql = "CREATE TABLE test_arrays_write (id INTEGER NOT NULL PRIMARY KEY, array JSONARRAY NOT NULL, strings JSONSTRINGLIST NOT NULL, ints JSONINTEGERLIST NOT NULL, reals JSONREALLIST NOT NULL)"
178178
cur.execute(sql)
179179
sql = "SELECT AddGeometryColumn('test_arrays_write', 'Geometry', 4326, 'POLYGON', 'XY')"
180180
cur.execute(sql)
@@ -281,6 +281,14 @@ def setUpClass(cls):
281281
"""
282282
cur.execute(sql)
283283

284+
# Transaction tables
285+
sql = "CREATE TABLE \"test_transactions1\"(pkuid integer primary key autoincrement)"
286+
cur.execute(sql)
287+
sql = "CREATE TABLE \"test_transactions2\"(pkuid integer primary key autoincrement)"
288+
cur.execute(sql)
289+
sql = "INSERT INTO \"test_transactions2\" VALUES (NULL)"
290+
cur.execute(sql)
291+
284292
# Commit all test data
285293
cur.execute("COMMIT")
286294
con.close()
@@ -1592,6 +1600,32 @@ def testTransaction(self):
15921600
len([f for f in vl2_external.getFeatures(QgsFeatureRequest())]), 1)
15931601
del vl2_external
15941602

1603+
def testTransactions(self):
1604+
"""Test autogenerate"""
1605+
1606+
vl = QgsVectorLayer("dbname=%s table=test_transactions1 ()" %
1607+
self.dbname, "test_transactions1", "spatialite")
1608+
self.assertTrue(vl.isValid())
1609+
vl2 = QgsVectorLayer("dbname=%s table=test_transactions2 ()" %
1610+
self.dbname, "test_transactions2", "spatialite")
1611+
self.assertTrue(vl.isValid())
1612+
self.assertTrue(vl2.isValid())
1613+
self.assertEqual(vl.featureCount(), 0)
1614+
self.assertEqual(vl2.featureCount(), 1)
1615+
1616+
project = QgsProject()
1617+
project.setAutoTransaction(True)
1618+
project.addMapLayers([vl, vl2])
1619+
project.setEvaluateDefaultValues(True)
1620+
1621+
self.assertTrue(vl.startEditing())
1622+
1623+
self.assertEqual(vl2.dataProvider().defaultValueClause(0), '')
1624+
self.assertEqual(vl2.dataProvider().defaultValue(0), 2)
1625+
1626+
self.assertEqual(vl.dataProvider().defaultValueClause(0), '')
1627+
self.assertEqual(vl.dataProvider().defaultValue(0), 1)
1628+
15951629

15961630
if __name__ == '__main__':
15971631
unittest.main()

0 commit comments

Comments
 (0)
Please sign in to comment.