Make sure sqlite_fetch_and_increment is always executed on the main t…

…hread
qgis · Jan 10, 2019 · a740fec · a740fec
1 parent dafb166
commit a740fec
Showing 1 changed file with 123 additions and 111 deletions.
diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp
@@ -49,6 +49,7 @@
 #include "qgsproviderregistry.h"
 #include "sqlite3.h"
 #include "qgstransaction.h"
+#include "qgsthreadingutils.h"
 
 const QString QgsExpressionFunction::helpText() const
 {
@@ -1377,151 +1378,162 @@ 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;
+  QVariant functionResult;
 
-  QString database;
-  const QgsVectorLayer *layer = QgsExpressionUtils::getVectorLayer( values.at( 0 ), parent );
-
-  if ( layer )
+  std::function<void()> fetchAndIncrementFunc = [ =, &functionResult ]()
   {
-    const QVariantMap decodedUri = QgsProviderRegistry::instance()->decodeUri( layer->providerType(), layer->dataProvider()->dataSourceUri() );
-    database = decodedUri.value( QStringLiteral( "path" ) ).toString();
-    if ( database.isEmpty() )
+    QString database;
+    const QgsVectorLayer *layer = QgsExpressionUtils::getVectorLayer( values.at( 0 ), parent );
+
+    if ( layer )
     {
-      parent->setEvalErrorString( QObject::tr( "Could not extract file path from layer `%1`." ).arg( layer->name() ) );
+      const QVariantMap decodedUri = QgsProviderRegistry::instance()->decodeUri( layer->providerType(), layer->dataProvider()->dataSourceUri() );
+      database = decodedUri.value( QStringLiteral( "path" ) ).toString();
+      if ( database.isEmpty() )
+      {
+        parent->setEvalErrorString( QObject::tr( "Could not extract file path from layer `%1`." ).arg( layer->name() ) );
+      }
+    }
+    else
+    {
+      database = values.at( 0 ).toString();
     }
-  }
-  else
-  {
-    database = values.at( 0 ).toString();
-  }
-
-  const QString table = values.at( 1 ).toString();
-  const QString idColumn = values.at( 2 ).toString();
-  const QString filterAttribute = values.at( 3 ).toString();
-  const QVariant filterValue = values.at( 4 ).toString();
-  const QVariantMap defaultValues = values.at( 5 ).toMap();
-
-  // read from database
-  sqlite3_database_unique_ptr sqliteDb;
-  sqlite3_statement_unique_ptr sqliteStatement;
 
-  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();
-  }
+    const QString table = values.at( 1 ).toString();
+    const QString idColumn = values.at( 2 ).toString();
+    const QString filterAttribute = values.at( 3 ).toString();
+    const QVariant filterValue = values.at( 4 ).toString();
+    const QVariantMap defaultValues = values.at( 5 ).toMap();
 
-  QString errorMessage;
-  QString currentValSql;
+    // read from database
+    sqlite3_database_unique_ptr sqliteDb;
+    sqlite3_statement_unique_ptr sqliteStatement;
 
-  qlonglong nextId;
-  bool cachedMode = false;
-  bool valueRetrieved = false;
+    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() ) );
+      functionResult = QVariant();
+      return;
+    }
 
-  QString cacheString = QStringLiteral( "%1:%2:%3:%4:%5" ).arg( database, table, idColumn, filterAttribute, filterValue.toString() );
+    QString errorMessage;
+    QString currentValSql;
 
-  // Running in transaction mode, check for cached value first
-  if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
-  {
-    cachedMode = true;
+    qlonglong nextId;
+    bool cachedMode = false;
+    bool valueRetrieved = false;
 
-    auto cachedCounter = counterCache.find( cacheString );
+    QString cacheString = QStringLiteral( "%1:%2:%3:%4:%5" ).arg( database, table, idColumn, filterAttribute, filterValue.toString() );
 
-    if ( cachedCounter != counterCache.end() )
+    // Running in transaction mode, check for cached value first
+    if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
     {
-      qlonglong &cachedValue = cachedCounter.value();
-      nextId = cachedValue;
-      nextId += 1;
-      cachedValue = nextId;
-      valueRetrieved = true;
-    }
-  }
+      cachedMode = true;
 
-  // Either not in cached mode or no cached value found, obtain from DB
-  if ( !cachedMode || !valueRetrieved )
-  {
-    int result = SQLITE_ERROR;
+      auto cachedCounter = counterCache.find( cacheString );
 
-    currentValSql = QStringLiteral( "SELECT %1 FROM %2" ).arg( QgsSqliteUtils::quotedIdentifier( idColumn ), QgsSqliteUtils::quotedIdentifier( table ) );
-    if ( !filterAttribute.isNull() )
-    {
-      currentValSql += QStringLiteral( " WHERE %1 = %2" ).arg( QgsSqliteUtils::quotedIdentifier( filterAttribute ), QgsSqliteUtils::quotedValue( filterValue ) );
+      if ( cachedCounter != counterCache.end() )
+      {
+        qlonglong &cachedValue = cachedCounter.value();
+        nextId = cachedValue;
+        nextId += 1;
+        cachedValue = nextId;
+        valueRetrieved = true;
+      }
     }
 
-    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 )
     {
-      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 ) );
       }
 
-      // If in cached mode: add value to cache and connect to transaction
-      if ( cachedMode && result == SQLITE_OK )
+      sqliteStatement = sqliteDb.prepare( currentValSql, result );
+
+      if ( result == SQLITE_OK )
       {
-        counterCache.insert( cacheString, nextId );
+        nextId = 0;
+        if ( sqliteStatement.step() == SQLITE_ROW )
+        {
+          nextId = sqliteStatement.columnAsInt64( 0 ) + 1;
+        }
 
-        QObject::connect( layer->dataProvider()->transaction(), &QgsTransaction::destroyed, [cacheString]()
+        // If in cached mode: add value to cache and connect to transaction
+        if ( cachedMode && result == SQLITE_OK )
         {
-          counterCache.remove( cacheString );
-        } );
+          counterCache.insert( cacheString, nextId );
+
+          QObject::connect( layer->dataProvider()->transaction(), &QgsTransaction::destroyed, [cacheString]()
+          {
+            counterCache.remove( cacheString );
+          } );
+        }
+        valueRetrieved = true;
       }
-      valueRetrieved = true;
     }
-  }
 
-  if ( valueRetrieved )
-  {
-    QString upsertSql;
-    upsertSql = QStringLiteral( "INSERT OR REPLACE INTO %1" ).arg( QgsSqliteUtils::quotedIdentifier( table ) );
-    QStringList cols;
-    QStringList vals;
-    cols << QgsSqliteUtils::quotedIdentifier( idColumn );
-    vals << QgsSqliteUtils::quotedValue( nextId );
-
-    if ( !filterAttribute.isNull() )
+    if ( valueRetrieved )
     {
-      cols << QgsSqliteUtils::quotedIdentifier( filterAttribute );
-      vals << QgsSqliteUtils::quotedValue( filterValue );
-    }
+      QString upsertSql;
+      upsertSql = QStringLiteral( "INSERT OR REPLACE INTO %1" ).arg( QgsSqliteUtils::quotedIdentifier( table ) );
+      QStringList cols;
+      QStringList vals;
+      cols << QgsSqliteUtils::quotedIdentifier( idColumn );
+      vals << QgsSqliteUtils::quotedValue( nextId );
+
+      if ( !filterAttribute.isNull() )
+      {
+        cols << QgsSqliteUtils::quotedIdentifier( filterAttribute );
+        vals << QgsSqliteUtils::quotedValue( filterValue );
+      }
 
-    for ( QVariantMap::const_iterator iter = defaultValues.constBegin(); iter != defaultValues.constEnd(); ++iter )
-    {
-      cols << QgsSqliteUtils::quotedIdentifier( iter.key() );
-      vals << iter.value().toString();
-    }
+      for ( QVariantMap::const_iterator iter = defaultValues.constBegin(); iter != defaultValues.constEnd(); ++iter )
+      {
+        cols << QgsSqliteUtils::quotedIdentifier( iter.key() );
+        vals << iter.value().toString();
+      }
 
-    upsertSql += QLatin1String( " (" ) + cols.join( ',' ) + ')';
-    upsertSql += QLatin1String( " VALUES " );
-    upsertSql += '(' + vals.join( ',' ) + ')';
+      upsertSql += QLatin1String( " (" ) + cols.join( ',' ) + ')';
+      upsertSql += QLatin1String( " VALUES " );
+      upsertSql += '(' + vals.join( ',' ) + ')';
 
-    int result = SQLITE_ERROR;
-    if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
-    {
-      QgsTransaction *transaction = layer->dataProvider()->transaction();
-      if ( transaction->executeSql( upsertSql, errorMessage ) )
+      int result = SQLITE_ERROR;
+      if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
       {
-        result = SQLITE_OK;
+        QgsTransaction *transaction = layer->dataProvider()->transaction();
+        if ( transaction->executeSql( upsertSql, errorMessage ) )
+        {
+          result = SQLITE_OK;
+        }
+      }
+      else
+      {
+        result = sqliteDb.exec( upsertSql, errorMessage );
+      }
+      if ( result == SQLITE_OK )
+      {
+        functionResult = QVariant( nextId );
+        return;
+      }
+      else
+      {
+        parent->setEvalErrorString( QStringLiteral( "Could not increment value: SQLite error: \"%1\" (%2)." ).arg( errorMessage, QString::number( result ) ) );
+        functionResult = QVariant();
+        return;
       }
     }
-    else
-    {
-      result = sqliteDb.exec( upsertSql, errorMessage );
-    }
-    if ( result == SQLITE_OK )
-    {
-      return nextId;
-    }
-    else
-    {
-      parent->setEvalErrorString( QStringLiteral( "Could not increment value: SQLite error: \"%1\" (%2)." ).arg( errorMessage, QString::number( result ) ) );
-      return QVariant();
-    }
-  }
 
-  return QVariant(); // really?
+    functionResult = QVariant();
+  };
+
+  QgsThreadingUtils::runOnMainThread( fetchAndIncrementFunc );
+
+  return functionResult;
 }
 
 static QVariant fcnConcat( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )