executing query after cloned connection is used in another thread

SGaist

Adding threads to an application doesn't mean you are going to get a sudden boost in all use cases. If the method you move to the thread was already really fast, then you won't notice much.

user4592357

@sgaist
i don't think the method was fast. this is the method in short: i need to update three tables. the how part depends on whether a row in each table exists. so i do "if exists, update. else, insert" a few times in the same function.
by the way, should i perform the "is exists then update else insert" part in c++ code, or it should be in a query? I'd say in query because the operations might be optimized at database level. but currently I'm doing them in c++ code

actually, i don't need the method in thread to execute immediately. i just need the gui not to freeze

SGaist

Indeed, let the database do that kind of stuff since it's made for that.

user4592357

@sgaist
okay. and one thing i don't get is, the function that does the updating is called from a menu (action). now when the function is executed is executed in another thread I'd expect the menu to close, by it is not.

here is another thing. the gui action calls a function, and in this function i instruct the thread to call the internal update function. should i do this? or call the first function initiated by the gui action from the new thread?

user4592357

okay so o tried to call qApp->processEvents() in the loop that updates the db. and i got rid of the thread. what about this solution?

for (auto row : rows)
{
	// update the db for the table row
	// ...

	// process pending events
	QCoreApplication::processEvents();
}

SGaist

@user4592357 said in executing query after cloned connection is used in another thread:

@sgaist
okay. and one thing i don't get is, the function that does the updating is called from a menu (action). now when the function is executed is executed in another thread I'd expect the menu to close, by it is not.

here is another thing. the gui action calls a function, and in this function i instruct the thread to call the internal update function. should i do this? or call the first function initiated by the gui action from the new thread?

Since I don't know how you implemented the thread more the menu setup, I can't comment on that.

user4592357

@sgaist
okay and what about the process events solution?

SGaist

Using processEvents is a last resort mesure that should be avoided.

user4592357

@sgaist
okay so it means i need to go back to my thread implementation
here's what i have

bool doWork(const QString &newVal)
{
     for (each of selected table rows)
     {
          if (!table->updateRow(row, newVal))
              return false;
          // QCoreApplication::processEvents();
     }
    return true;
}
bool Table::updateRow(int row, const QString &newVal)
{
     m_thread.execute(updateQuery, row, newVal);
     //...
}

when the processEvents is commented, the gui freezes, since the call to updateRow is executed in gui thread.
what should i do? put the for loop of doWork() in the thread? (the table is generic and in my case i need to update the database but another table might do something else where multithreaded update is not needed... although it won't hurt maybe)

SGaist

How proficient are you in the threading field ?

Because from the looks of it, you are directly calling methods on your thread class and expecting them to actually run in that other thread. It's not how it works.

user4592357

@sgaist
sorry for no context. i create the database and then use slots to execute the queries (because i want to execute the queries on some event, which is when the user decided to modify the data in my table widget).
roughly speaking, this is my implementation:

#pragma once

#include <QThread>
#include <QDebug>
#include <QtSql/QSqlError>
#include <QtSql/QSqlDatabase>
#include <QtSql/QSqlQuery>

class queryExecutor : public QObject
{
	Q_OBJECT
public:
	explicit queryExecutor(QObject *parent = nullptr) noexcept;

	void setDatabase(const QString &sDbPath);

public slots:
	void execute(const QString &query, const QHash<QString, QString> &mapBoundVals);

signals:
	void finished(bool success, const QList<QSqlRecord> &records);

private:
	QSqlDatabase m_db;
};

class QueryThread : public QThread
{
	Q_OBJECT
public:
	explicit QueryThread(const QString &sDbPath) noexcept;

protected:
	void run() override;

signals:
	void queryFinished(bool success, const QList<QSqlRecord> &records);

public slots:
	void execute(const QString &query, const QHash<QString, QString> &mapBoundVals);

private:
	queryExecutor m_queryExecutor;
	QString m_sDbPath;
};

queryExecutor::queryExecutor(QObject *parent /* = nullptr */) noexcept
{
}

void queryExecutor::setDatabase(const QString &sDbPath)
{
	// create a thread-specific connection
	m_db = QSqlDatabase::addDatabase("QSQLITE", QString("%1_%2")
		.arg(sDbPath)
		.arg(QString::number((quint64)QThread::currentThreadId(), 16)));

	qDebug() << "conn name" << QString("%1_%2")
		.arg(sDbPath)
		.arg(QString::number((quint64)QThread::currentThreadId(), 16));

	m_db.setDatabaseName(sDbPath);

	if (!m_db.open())
	{
		qDebug() << "db not open:" << m_db.lastError().text();
	}
}

void queryExecutor::execute(const QString &query, const QHash<QString, QString> &mapBoundVals)
{
	QList<QSqlRecord> records;
	QSqlQuery sqlQuery(m_db);
	sqlQuery.prepare(query);

	for (const auto &sPlaceholder : mapBoundVals.keys())
	{
		auto bound = mapBoundVals[sPlaceholder];
		sqlQuery.bindValue(sPlaceholder, bound);
	}

	if (!sqlQuery.exec())
	{
		qDebug() << "query failed:" << sqlQuery.lastError().text();
		emit finished(false, records);
		return;
	}

	while (sqlQuery.next())
		records.push_back(sqlQuery.record());

	emit finished(true, records);
}

QueryThread::QueryThread(const QString &sDbPath) noexcept
	: m_sDbPath(sDbPath)
{
}

void QueryThread::execute(const QString &query, const QHash<QString, QString> &mapBoundVals)
{
	// forward the execution to the worker
	m_queryExecutor.execute(query, mapBoundVals);
}

void QueryThread::run()
{
	qDebug() << "started thread" << QThread::currentThreadId();

	m_queryExecutor.setDatabase(m_sDbPath);

	// forward final signal
	connect(&m_queryExecutor, &queryExecutor::finished, this, &QueryThread::queryFinished);

	exec();
}

Christian Ehrlicher

Your function queryExecutor::execute() is not run in your thread since the slot from where it's called is a function from an object which lives in the main thread. I would suggest reading the QThread documentation on how to properly use Signals/Slots with QThread: https://doc.qt.io/qt-5/threads-qobject.html

user4592357

@christian-ehrlicher
yeah that's right. query executor should execute the query since it is in the new thread, so the code should be like this:

void QueryThread::execute(const QString &query, const QHash<QString, QString> &mapBoundVals)
{
	// forward the execution to the worker
	emit executefwd(query, mapBoundVals);
}

void QueryThread::run()
{
	qDebug() << "started thread" << QThread::currentThreadId();

	m_queryExecutor.setDatabase(m_sDbPath);

        connect( this, SIGNAL( executefwd( const QString&, const QHash<QString, QString> & ) ),
             m_queryExecutor, SLOT( execute( const QString&, const QHash<QString, QString> & ) ) );

	// forward final signal
	connect(&m_queryExecutor, &queryExecutor::finished, this, &QueryThread::queryFinished);

	exec();
}

SGaist

Your executor still lives in the main thread as it is created at your custom thread subclass creation time.

user4592357

@sgaist
oh okay, so it should be newed in QueryThread::run()

SGaist

It can be on the stack. It really depends on how you want to implement that stuff. You seem to be mixing the worker object approach with the subclass QThread approach.

user4592357

@sgaist
i followed this example for my implementation: https://www.linuxjournal.com/article/9602
but anyways i think thread is still not much helpful because my for loop still executes in main thread. am i right?

bool doWork(const QString &newVal)
{
     for (each of selected table rows)
     {
          if (!table->updateRow(row, newVal))
              return false;
     }
    return true;
}

bool Table::updateRow(int row, const QString &newVal)
{
     //update in another thread
}

SGaist

As already said, you are mixing two different technique the wrong way.

If you subclass QThread and re-implement run, then what is executed in run happens in the thread managed by QThread.

However, you created an instance of your worker object in the original thread since it's part of your QThread subclass and thus the slot will be executed in the original thread.