Add cached mode to sqlite_fetch_and_increment for transactions

qgis · Jan 10, 2019 · d3b4c7d · d3b4c7d
1 parent 918bfef
commit d3b4c7d
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 61 deletions.
diff --git a/resources/function_help/json/sqlite_fetch_and_increment b/resources/function_help/json/sqlite_fetch_and_increment
@@ -1,9 +1,9 @@
 {
   "name": "sqlite_fetch_and_increment",
   "type": "function",
-  "description": "Manage autoincrementing values in sqlite databases.<p>SQlite default values can only be applied on insert and not prefetched.</p><p>This makes it impossible to acquire an incremented primary key via AUTO_INCREMENT before creating the row in the database. Sidenote: with postgres, this works via the option <i>evaluate default values</i>.</p><p>When adding new features with relations, it is really nice to be able to already add children for a parent, while the parents form is still open and hence the parent feature uncommitted.</p><p>To get around this limitation, this function can be used to manage sequence values in a separate table on sqlite based formats like gpkg.</p><p>The sequence table will be filtered for a sequence id (filter_attribute and filter_value) and the current value of the id_field will be incremented by 1 ond the incremented value returned.</p><p>If additional columns require values to be specified, the default_value map can be used for this purpose.</p><p><b>Note</b><br/>This function modifies the target sqlite table. It is intended for usage with default value configurations for attributes.</p>",
+  "description": "Manage autoincrementing values in sqlite databases.<p>SQlite default values can only be applied on insert and not prefetched.</p><p>This makes it impossible to acquire an incremented primary key via AUTO_INCREMENT before creating the row in the database. Sidenote: with postgres, this works via the option <i>evaluate default values</i>.</p><p>When adding new features with relations, it is really nice to be able to already add children for a parent, while the parents form is still open and hence the parent feature uncommitted.</p><p>To get around this limitation, this function can be used to manage sequence values in a separate table on sqlite based formats like gpkg.</p><p>The sequence table will be filtered for a sequence id (filter_attribute and filter_value) and the current value of the id_field will be incremented by 1 ond the incremented value returned.</p><p>If additional columns require values to be specified, the default_value map can be used for this purpose.</p><p><b>Note</b><br/>This function modifies the target sqlite table. It is intended for usage with default value configurations for attributes.</p><p>When the database parameter is a layer and the layer is in transaction mode, the value will only be retrieved once during the lifetime of a transaction and cached and incremented. This makes it unsafe to work on the same database from several processes in parallel.</p>",
   "arguments": [
-	  {"arg":"database", "description":"Path to the sqlite file"},
+	  {"arg":"database", "description":"Path to the sqlite file or layer"},
     {"arg":"table", "description":"Name of the table that manages the sequences"},
     {"arg":"id_field", "description":"Name of the field that contains the current value"},
     {"arg":"filter_attribute", "description":"Name the field that contains a unique identifier for this sequence. Must have a UNIQUE index."},

diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp
@@ -1376,6 +1376,8 @@ static QVariant fcnNumSelected( const QVariantList &values, const QgsExpressionC
 
 static QVariant fcnSqliteFetchAndIncrement( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )
 {
+  static QMap<QString, qlonglong> counterCache;
+
   QString database;
   const QgsVectorLayer *layer = QgsExpressionUtils::getVectorLayer( values.at( 0 ), parent );
 
@@ -1403,38 +1405,75 @@ static QVariant fcnSqliteFetchAndIncrement( const QVariantList &values, const Qg
   sqlite3_database_unique_ptr sqliteDb;
   sqlite3_statement_unique_ptr sqliteStatement;
 
-  if ( sqliteDb.open_v2( database, SQLITE_OPEN_READWRITE | SQLITE_OPEN_SHAREDCACHE, nullptr ) != SQLITE_OK )
+  if ( sqliteDb.open_v2( database, SQLITE_OPEN_READWRITE, nullptr ) != SQLITE_OK )
   {
     parent->setEvalErrorString( QObject::tr( "Could not open sqlite database %1. Error %2. " ).arg( database, sqliteDb.errorMessage() ) );
     return QVariant();
   }
 
-  QString enableReadUncommitted = QStringLiteral( "PRAGMA read_uncommitted = true;" );
-
   QString errorMessage;
-  if ( sqliteDb.exec( enableReadUncommitted, errorMessage ) != SQLITE_OK )
-  {
-    // TODO HANDLE AN ERROR
-    Q_ASSERT( false );
-  }
-
   QString currentValSql;
-  currentValSql = QStringLiteral( "SELECT %1 FROM %2" ).arg( QgsSqliteUtils::quotedIdentifier( idColumn ), QgsSqliteUtils::quotedIdentifier( table ) );
-  if ( !filterAttribute.isNull() )
+
+  qlonglong nextId;
+  bool cachedMode = false;
+  bool valueRetrieved = false;
+
+  QString cacheString = QStringLiteral( "%1:%2:%3:%4:%5" ).arg( database, table, idColumn, filterAttribute, filterValue.toString() );
+
+  // Running in transaction mode, check for cached value first
+  if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
   {
-    currentValSql += QStringLiteral( " WHERE %1 = %2" ).arg( QgsSqliteUtils::quotedIdentifier( filterAttribute ), QgsSqliteUtils::quotedValue( filterValue ) );
+    cachedMode = true;
+
+    auto cachedCounter = counterCache.find( cacheString );
+
+    if ( cachedCounter != counterCache.end() )
+    {
+      qlonglong &cachedValue = cachedCounter.value();
+      nextId = cachedValue;
+      nextId += 1;
+      cachedValue = nextId;
+      valueRetrieved = true;
+    }
   }
 
-  int result = SQLITE_ERROR;
-  sqliteStatement = sqliteDb.prepare( currentValSql, result );
-  if ( result == SQLITE_OK )
+  // Either not in cached mode or no cached value found, obtain from DB
+  if ( !cachedMode || !valueRetrieved )
   {
-    qlonglong nextId = 0;
-    if ( sqliteStatement.step() == SQLITE_ROW )
+    int result = SQLITE_ERROR;
+
+    currentValSql = QStringLiteral( "SELECT %1 FROM %2" ).arg( QgsSqliteUtils::quotedIdentifier( idColumn ), QgsSqliteUtils::quotedIdentifier( table ) );
+    if ( !filterAttribute.isNull() )
     {
-      nextId = sqliteStatement.columnAsInt64( 0 ) + 1;
+      currentValSql += QStringLiteral( " WHERE %1 = %2" ).arg( QgsSqliteUtils::quotedIdentifier( filterAttribute ), QgsSqliteUtils::quotedValue( filterValue ) );
     }
 
+    sqliteStatement = sqliteDb.prepare( currentValSql, result );
+
+    if ( result == SQLITE_OK )
+    {
+      nextId = 0;
+      if ( sqliteStatement.step() == SQLITE_ROW )
+      {
+        nextId = sqliteStatement.columnAsInt64( 0 ) + 1;
+      }
+
+      // If in cached mode: add value to cache and connect to transaction
+      if ( cachedMode && result == SQLITE_OK )
+      {
+        counterCache.insert( cacheString, nextId );
+
+        QObject::connect( layer->dataProvider()->transaction(), &QgsTransaction::destroyed, [cacheString]()
+        {
+          counterCache.remove( cacheString );
+        } );
+      }
+      valueRetrieved = true;
+    }
+  }
+
+  if ( valueRetrieved )
+  {
     QString upsertSql;
     upsertSql = QStringLiteral( "INSERT OR REPLACE INTO %1" ).arg( QgsSqliteUtils::quotedIdentifier( table ) );
     QStringList cols;
@@ -1458,6 +1497,7 @@ static QVariant fcnSqliteFetchAndIncrement( const QVariantList &values, const Qg
     upsertSql += QLatin1String( " VALUES " );
     upsertSql += '(' + vals.join( ',' ) + ')';
 
+    int result = SQLITE_ERROR;
     if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
     {
       QgsTransaction *transaction = layer->dataProvider()->transaction();

diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -33,6 +33,7 @@
 #include "qgsrasterlayer.h"
 #include "qgsproject.h"
 #include "qgsexpressionnodeimpl.h"
+#include "qgsvectorlayerutils.h"
 
 static void _parseAndEvalExpr( int arg )
 {
@@ -3298,6 +3299,43 @@ class TestQgsExpression: public QObject
       e = QgsExpression( QStringLiteral( "map('a',1,'bbb',2,'c',3)['b'||'b'||'b']" ) );
       QCOMPARE( e.evaluate( &context ).toInt(), 2 );
     }
+
+
+    void testSqliteFetchAndIncrementWithTranscationMode()
+    {
+      QString testDataDir = QStringLiteral( TEST_DATA_DIR ) + '/';
+      QTemporaryDir tempDir;
+      QFile::copy( testDataDir + QStringLiteral( "kbs.qgs" ), tempDir.filePath( "kbs.qgs" ) );
+      QFile::copy( testDataDir + QStringLiteral( "kbs.gpkg" ), tempDir.filePath( "kbs.gpkg" ) );
+
+      QgsProject *project = QgsProject::instance();
+      QVERIFY( project->read( tempDir.filePath( "kbs.qgs" ) ) );
+
+      QgsVectorLayer *zustaendigkeitskataster = project->mapLayer<QgsVectorLayer *>( QStringLiteral( "zustaendigkeitkataster_2b5bb693_3151_4c82_967f_b49d4d348a17" ) );
+
+      // There is a default expression setup, dear reader of this test
+      QVERIFY( zustaendigkeitskataster->defaultValueDefinition( 0 ).expression().contains( "sqlite_fetch_and_increment" ) );
+
+      zustaendigkeitskataster->startEditing();
+
+      QgsExpressionContext context( QgsExpressionContextUtils::globalProjectLayerScopes( zustaendigkeitskataster ) );
+      QgsFeature feature = QgsVectorLayerUtils::createFeature( zustaendigkeitskataster, QgsGeometry(), QgsAttributeMap(), &context );
+      QCOMPARE( feature.attribute( "T_Id" ), 0 );
+      feature.setAttribute( "url_behoerde", "url_behoerde" );
+      feature.setAttribute( "url_kataster", "url_kataster" );
+      zustaendigkeitskataster->addFeature( feature );
+
+      QgsFeature feature2 = QgsVectorLayerUtils::createFeature( zustaendigkeitskataster, QgsGeometry(), QgsAttributeMap(), &context );
+      QCOMPARE( feature2.attribute( "T_Id" ), 1 );
+      feature2.setAttribute( "url_behoerde", "url_behoerde_x" );
+      feature2.setAttribute( "url_kataster", "url_kataster_x" );
+      zustaendigkeitskataster->addFeature( feature2 );
+
+      zustaendigkeitskataster->commitChanges();
+
+      QCOMPARE( zustaendigkeitskataster->dataProvider()->featureCount(), 2 );
+    }
+
 };
 
 QGSTEST_MAIN( TestQgsExpression )

diff --git a/tests/src/providers/testqgsogrprovider.cpp b/tests/src/providers/testqgsogrprovider.cpp
@@ -24,10 +24,6 @@
 #include <qgsproviderregistry.h>
 #include <qgsvectorlayer.h>
 #include <qgsnetworkaccessmanager.h>
-#include <qgsproject.h>
-#include <qgsvectorlayerutils.h>
-#include <qgstransactiongroup.h>
-#include <sqlite3.h>
 
 #include <QObject>
 
@@ -50,7 +46,6 @@ class TestQgsOgrProvider : public QObject
 
     void setupProxy();
     void decodeUri();
-    void testGpkgTransactions();
 
   private:
     QString mTestDataDir;
@@ -135,41 +130,5 @@ void TestQgsOgrProvider::decodeUri()
   QCOMPARE( parts.value( QStringLiteral( "layerName" ) ).toString(), QString( "a_layer" ) );
 }
 
-void TestQgsOgrProvider::testGpkgTransactions()
-{
-  QTemporaryDir tempDir;
-  QFile::copy( mTestDataDir + QStringLiteral( "kbs.qgs" ), tempDir.filePath( "kbs.qgs" ) );
-  QFile::copy( mTestDataDir + QStringLiteral( "kbs.gpkg" ), tempDir.filePath( "kbs.gpkg" ) );
-
-  sqlite3_enable_shared_cache( 1 );
-  QgsProject *project = QgsProject::instance();
-  QVERIFY( project->read( tempDir.filePath( "kbs.qgs" ) ) );
-
-  QgsVectorLayer *zustaendigkeitskataster = project->mapLayer<QgsVectorLayer *>( QStringLiteral( "zustaendigkeitkataster_2b5bb693_3151_4c82_967f_b49d4d348a17" ) );
-
-  // There is a default expression setup, dear reader of this test
-  QVERIFY( zustaendigkeitskataster->defaultValueDefinition( 0 ).expression().contains( "sqlite_fetch_and_increment" ) );
-
-  zustaendigkeitskataster->startEditing();
-
-  QgsExpressionContext context( QgsExpressionContextUtils::globalProjectLayerScopes( zustaendigkeitskataster ) );
-  QgsFeature feature = QgsVectorLayerUtils::createFeature( zustaendigkeitskataster, QgsGeometry(), QgsAttributeMap(), &context );
-  QCOMPARE( feature.attribute( "T_Id" ), 0 );
-  feature.setAttribute( "url_behoerde", "url_behoerde" );
-  feature.setAttribute( "url_kataster", "url_kataster" );
-  zustaendigkeitskataster->addFeature( feature );
-
-  QgsFeature feature2 = QgsVectorLayerUtils::createFeature( zustaendigkeitskataster, QgsGeometry(), QgsAttributeMap(), &context );
-  QCOMPARE( feature2.attribute( "T_Id" ), 1 );
-  feature2.setAttribute( "url_behoerde", "url_behoerde_x" );
-  feature2.setAttribute( "url_kataster", "url_kataster_x" );
-  zustaendigkeitskataster->addFeature( feature2 );
-
-  zustaendigkeitskataster->commitChanges();
-
-  QCOMPARE( zustaendigkeitskataster->dataProvider()->featureCount(), 2 );
-}
-
-
 QGSTEST_MAIN( TestQgsOgrProvider )
 #include "testqgsogrprovider.moc"