Skip to content

Commit a740fec

Browse files
m-kuhnm-kuhn
committedJan 10, 2019
Make sure sqlite_fetch_and_increment is always executed on the main thread
·
ltr-3_40final-3_6_0
1 parent dafb166 commit a740fec

File tree

1 file changed

+123
-111
lines changed

1 file changed

+123
-111
lines changed
 

‎src/core/expression/qgsexpressionfunction.cpp

Lines changed: 123 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
#include "qgsproviderregistry.h"
5050
#include "sqlite3.h"
5151
#include "qgstransaction.h"
52+
#include "qgsthreadingutils.h"
5253

5354
const QString QgsExpressionFunction::helpText() const
5455
{
@@ -1377,151 +1378,162 @@ static QVariant fcnNumSelected( const QVariantList &values, const QgsExpressionC
13771378
static QVariant fcnSqliteFetchAndIncrement( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )
13781379
{
13791380
static QMap<QString, qlonglong> counterCache;
1381+
QVariant functionResult;
13801382

1381-
QString database;
1382-
const QgsVectorLayer *layer = QgsExpressionUtils::getVectorLayer( values.at( 0 ), parent );
1383-
1384-
if ( layer )
1383+
std::function<void()> fetchAndIncrementFunc = [ =, &functionResult ]()
13851384
{
1386-
const QVariantMap decodedUri = QgsProviderRegistry::instance()->decodeUri( layer->providerType(), layer->dataProvider()->dataSourceUri() );
1387-
database = decodedUri.value( QStringLiteral( "path" ) ).toString();
1388-
if ( database.isEmpty() )
1385+
QString database;
1386+
const QgsVectorLayer *layer = QgsExpressionUtils::getVectorLayer( values.at( 0 ), parent );
1387+
1388+
if ( layer )
13891389
{
1390-
parent->setEvalErrorString( QObject::tr( "Could not extract file path from layer `%1`." ).arg( layer->name() ) );
1390+
const QVariantMap decodedUri = QgsProviderRegistry::instance()->decodeUri( layer->providerType(), layer->dataProvider()->dataSourceUri() );
1391+
database = decodedUri.value( QStringLiteral( "path" ) ).toString();
1392+
if ( database.isEmpty() )
1393+
{
1394+
parent->setEvalErrorString( QObject::tr( "Could not extract file path from layer `%1`." ).arg( layer->name() ) );
1395+
}
1396+
}
1397+
else
1398+
{
1399+
database = values.at( 0 ).toString();
13911400
}
1392-
}
1393-
else
1394-
{
1395-
database = values.at( 0 ).toString();
1396-
}
1397-
1398-
const QString table = values.at( 1 ).toString();
1399-
const QString idColumn = values.at( 2 ).toString();
1400-
const QString filterAttribute = values.at( 3 ).toString();
1401-
const QVariant filterValue = values.at( 4 ).toString();
1402-
const QVariantMap defaultValues = values.at( 5 ).toMap();
1403-
1404-
// read from database
1405-
sqlite3_database_unique_ptr sqliteDb;
1406-
sqlite3_statement_unique_ptr sqliteStatement;
14071401

1408-
if ( sqliteDb.open_v2( database, SQLITE_OPEN_READWRITE, nullptr ) != SQLITE_OK )
1409-
{
1410-
parent->setEvalErrorString( QObject::tr( "Could not open sqlite database %1. Error %2. " ).arg( database, sqliteDb.errorMessage() ) );
1411-
return QVariant();
1412-
}
1402+
const QString table = values.at( 1 ).toString();
1403+
const QString idColumn = values.at( 2 ).toString();
1404+
const QString filterAttribute = values.at( 3 ).toString();
1405+
const QVariant filterValue = values.at( 4 ).toString();
1406+
const QVariantMap defaultValues = values.at( 5 ).toMap();
14131407

1414-
QString errorMessage;
1415-
QString currentValSql;
1408+
// read from database
1409+
sqlite3_database_unique_ptr sqliteDb;
1410+
sqlite3_statement_unique_ptr sqliteStatement;
14161411

1417-
qlonglong nextId;
1418-
bool cachedMode = false;
1419-
bool valueRetrieved = false;
1412+
if ( sqliteDb.open_v2( database, SQLITE_OPEN_READWRITE, nullptr ) != SQLITE_OK )
1413+
{
1414+
parent->setEvalErrorString( QObject::tr( "Could not open sqlite database %1. Error %2. " ).arg( database, sqliteDb.errorMessage() ) );
1415+
functionResult = QVariant();
1416+
return;
1417+
}
14201418

1421-
QString cacheString = QStringLiteral( "%1:%2:%3:%4:%5" ).arg( database, table, idColumn, filterAttribute, filterValue.toString() );
1419+
QString errorMessage;
1420+
QString currentValSql;
14221421

1423-
// Running in transaction mode, check for cached value first
1424-
if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
1425-
{
1426-
cachedMode = true;
1422+
qlonglong nextId;
1423+
bool cachedMode = false;
1424+
bool valueRetrieved = false;
14271425

1428-
auto cachedCounter = counterCache.find( cacheString );
1426+
QString cacheString = QStringLiteral( "%1:%2:%3:%4:%5" ).arg( database, table, idColumn, filterAttribute, filterValue.toString() );
14291427

1430-
if ( cachedCounter != counterCache.end() )
1428+
// Running in transaction mode, check for cached value first
1429+
if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
14311430
{
1432-
qlonglong &cachedValue = cachedCounter.value();
1433-
nextId = cachedValue;
1434-
nextId += 1;
1435-
cachedValue = nextId;
1436-
valueRetrieved = true;
1437-
}
1438-
}
1431+
cachedMode = true;
14391432

1440-
// Either not in cached mode or no cached value found, obtain from DB
1441-
if ( !cachedMode || !valueRetrieved )
1442-
{
1443-
int result = SQLITE_ERROR;
1433+
auto cachedCounter = counterCache.find( cacheString );
14441434

1445-
currentValSql = QStringLiteral( "SELECT %1 FROM %2" ).arg( QgsSqliteUtils::quotedIdentifier( idColumn ), QgsSqliteUtils::quotedIdentifier( table ) );
1446-
if ( !filterAttribute.isNull() )
1447-
{
1448-
currentValSql += QStringLiteral( " WHERE %1 = %2" ).arg( QgsSqliteUtils::quotedIdentifier( filterAttribute ), QgsSqliteUtils::quotedValue( filterValue ) );
1435+
if ( cachedCounter != counterCache.end() )
1436+
{
1437+
qlonglong &cachedValue = cachedCounter.value();
1438+
nextId = cachedValue;
1439+
nextId += 1;
1440+
cachedValue = nextId;
1441+
valueRetrieved = true;
1442+
}
14491443
}
14501444

1451-
sqliteStatement = sqliteDb.prepare( currentValSql, result );
1452-
1453-
if ( result == SQLITE_OK )
1445+
// Either not in cached mode or no cached value found, obtain from DB
1446+
if ( !cachedMode || !valueRetrieved )
14541447
{
1455-
nextId = 0;
1456-
if ( sqliteStatement.step() == SQLITE_ROW )
1448+
int result = SQLITE_ERROR;
1449+
1450+
currentValSql = QStringLiteral( "SELECT %1 FROM %2" ).arg( QgsSqliteUtils::quotedIdentifier( idColumn ), QgsSqliteUtils::quotedIdentifier( table ) );
1451+
if ( !filterAttribute.isNull() )
14571452
{
1458-
nextId = sqliteStatement.columnAsInt64( 0 ) + 1;
1453+
currentValSql += QStringLiteral( " WHERE %1 = %2" ).arg( QgsSqliteUtils::quotedIdentifier( filterAttribute ), QgsSqliteUtils::quotedValue( filterValue ) );
14591454
}
14601455

1461-
// If in cached mode: add value to cache and connect to transaction
1462-
if ( cachedMode && result == SQLITE_OK )
1456+
sqliteStatement = sqliteDb.prepare( currentValSql, result );
1457+
1458+
if ( result == SQLITE_OK )
14631459
{
1464-
counterCache.insert( cacheString, nextId );
1460+
nextId = 0;
1461+
if ( sqliteStatement.step() == SQLITE_ROW )
1462+
{
1463+
nextId = sqliteStatement.columnAsInt64( 0 ) + 1;
1464+
}
14651465

1466-
QObject::connect( layer->dataProvider()->transaction(), &QgsTransaction::destroyed, [cacheString]()
1466+
// If in cached mode: add value to cache and connect to transaction
1467+
if ( cachedMode && result == SQLITE_OK )
14671468
{
1468-
counterCache.remove( cacheString );
1469-
} );
1469+
counterCache.insert( cacheString, nextId );
1470+
1471+
QObject::connect( layer->dataProvider()->transaction(), &QgsTransaction::destroyed, [cacheString]()
1472+
{
1473+
counterCache.remove( cacheString );
1474+
} );
1475+
}
1476+
valueRetrieved = true;
14701477
}
1471-
valueRetrieved = true;
14721478
}
1473-
}
14741479

1475-
if ( valueRetrieved )
1476-
{
1477-
QString upsertSql;
1478-
upsertSql = QStringLiteral( "INSERT OR REPLACE INTO %1" ).arg( QgsSqliteUtils::quotedIdentifier( table ) );
1479-
QStringList cols;
1480-
QStringList vals;
1481-
cols << QgsSqliteUtils::quotedIdentifier( idColumn );
1482-
vals << QgsSqliteUtils::quotedValue( nextId );
1483-
1484-
if ( !filterAttribute.isNull() )
1480+
if ( valueRetrieved )
14851481
{
1486-
cols << QgsSqliteUtils::quotedIdentifier( filterAttribute );
1487-
vals << QgsSqliteUtils::quotedValue( filterValue );
1488-
}
1482+
QString upsertSql;
1483+
upsertSql = QStringLiteral( "INSERT OR REPLACE INTO %1" ).arg( QgsSqliteUtils::quotedIdentifier( table ) );
1484+
QStringList cols;
1485+
QStringList vals;
1486+
cols << QgsSqliteUtils::quotedIdentifier( idColumn );
1487+
vals << QgsSqliteUtils::quotedValue( nextId );
1488+
1489+
if ( !filterAttribute.isNull() )
1490+
{
1491+
cols << QgsSqliteUtils::quotedIdentifier( filterAttribute );
1492+
vals << QgsSqliteUtils::quotedValue( filterValue );
1493+
}
14891494

1490-
for ( QVariantMap::const_iterator iter = defaultValues.constBegin(); iter != defaultValues.constEnd(); ++iter )
1491-
{
1492-
cols << QgsSqliteUtils::quotedIdentifier( iter.key() );
1493-
vals << iter.value().toString();
1494-
}
1495+
for ( QVariantMap::const_iterator iter = defaultValues.constBegin(); iter != defaultValues.constEnd(); ++iter )
1496+
{
1497+
cols << QgsSqliteUtils::quotedIdentifier( iter.key() );
1498+
vals << iter.value().toString();
1499+
}
14951500

1496-
upsertSql += QLatin1String( " (" ) + cols.join( ',' ) + ')';
1497-
upsertSql += QLatin1String( " VALUES " );
1498-
upsertSql += '(' + vals.join( ',' ) + ')';
1501+
upsertSql += QLatin1String( " (" ) + cols.join( ',' ) + ')';
1502+
upsertSql += QLatin1String( " VALUES " );
1503+
upsertSql += '(' + vals.join( ',' ) + ')';
14991504

1500-
int result = SQLITE_ERROR;
1501-
if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
1502-
{
1503-
QgsTransaction *transaction = layer->dataProvider()->transaction();
1504-
if ( transaction->executeSql( upsertSql, errorMessage ) )
1505+
int result = SQLITE_ERROR;
1506+
if ( layer && layer->dataProvider() && layer->dataProvider()->transaction() )
15051507
{
1506-
result = SQLITE_OK;
1508+
QgsTransaction *transaction = layer->dataProvider()->transaction();
1509+
if ( transaction->executeSql( upsertSql, errorMessage ) )
1510+
{
1511+
result = SQLITE_OK;
1512+
}
1513+
}
1514+
else
1515+
{
1516+
result = sqliteDb.exec( upsertSql, errorMessage );
1517+
}
1518+
if ( result == SQLITE_OK )
1519+
{
1520+
functionResult = QVariant( nextId );
1521+
return;
1522+
}
1523+
else
1524+
{
1525+
parent->setEvalErrorString( QStringLiteral( "Could not increment value: SQLite error: \"%1\" (%2)." ).arg( errorMessage, QString::number( result ) ) );
1526+
functionResult = QVariant();
1527+
return;
15071528
}
15081529
}
1509-
else
1510-
{
1511-
result = sqliteDb.exec( upsertSql, errorMessage );
1512-
}
1513-
if ( result == SQLITE_OK )
1514-
{
1515-
return nextId;
1516-
}
1517-
else
1518-
{
1519-
parent->setEvalErrorString( QStringLiteral( "Could not increment value: SQLite error: \"%1\" (%2)." ).arg( errorMessage, QString::number( result ) ) );
1520-
return QVariant();
1521-
}
1522-
}
15231530

1524-
return QVariant(); // really?
1531+
functionResult = QVariant();
1532+
};
1533+
1534+
QgsThreadingUtils::runOnMainThread( fetchAndIncrementFunc );
1535+
1536+
return functionResult;
15251537
}
15261538

15271539
static QVariant fcnConcat( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )

0 commit comments

Comments
 (0)
Please sign in to comment.