Hello,
When testing our app with the newest Qt, I bumped into the error in the
subject (and nothing works :)).
How are we supposed to use SQLite :memory: databases from multiple
threads where each QSqlDatabase::addDatabase() call creates a new, empty db?
SQLite driver embedded in QtSql is compiled with THREADSAFE=1 [1][2]. So
it's safe to use it from multiple threads. I'm sure not all SQL drivers
come with thread safety guarantees like this, but limiting the
functionality of the whole module just because one or two database
backends are not threadsafe is not the right move.
This should be a big fat warning in the documentation but enforcing it
like this is too harsh.
We are all C++ programmers so we are supposed to know what we are doing
(hahaha :))) It's not Qt's job to protect us from our own choices.
Please revert this change or at least make it driver-specific so those
of us who use SQLite from multiple threads are don't have to patch Qt.
Best regards,
Burak
[1]: https://sqlite.org/compile.html#threadsafe
[2]: You can check it by running PRAGMA compile_options
Hi,
The QtSql classes themselves are not set to be reentrant or thread-safe so even if the SQL driver side is thread-safe there is no guarantee that the Qt side will be. What QtSql has always indicated is that you can use a QSqlDatabase to the same database in another thread but it has not been possible to share that object across threads, granted it may have worked by chance before but this was never guaranteed.
Granted the ":memory:" case is a unique one in this regard because opening another QSqlDatabase on ":memory:" will give you a new one, however it seems you can do:
":memory:?cache=shared"
To get it to access the same one. Does that help then?
Regards,
Andy
Interest på vegne av Burak Arslan <interest-bounces+andy.shaw=***@qt-project.org på vegne av ***@arskom.com.tr> skrev følgende den 20.08.2018, 09:07:
Hello,
When testing our app with the newest Qt, I bumped into the error in the
subject (and nothing works :)).
How are we supposed to use SQLite :memory: databases from multiple
threads where each QSqlDatabase::addDatabase() call creates a new, empty db?
SQLite driver embedded in QtSql is compiled with THREADSAFE=1 [1][2]. So
it's safe to use it from multiple threads. I'm sure not all SQL drivers
come with thread safety guarantees like this, but limiting the
functionality of the whole module just because one or two database
backends are not threadsafe is not the right move.
This should be a big fat warning in the documentation but enforcing it
like this is too harsh.
We are all C++ programmers so we are supposed to know what we are doing
(hahaha :))) It's not Qt's job to protect us from our own choices.
Please revert this change or at least make it driver-specific so those
of us who use SQLite from multiple threads are don't have to patch Qt.
Best regards,
Burak
[1]: https://sqlite.org/compile.html#threadsafe
[2]: You can check it by running PRAGMA compile_options
_______________________________________________
Interest mailing list
***@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest
Hey Andy,
Thanks for your reply.
Hi,
The QtSql classes themselves are not set to be reentrant or thread-safe (...) it may have worked by chance before but this was never guaranteed.
We the people can read the source code just as well as anyone and can
already see that. However, it should be our decision to use the library
the way we want.
I fully understand why making QSqlDatabase thread-safe may not be
feasible and just to be clear, I'm not asking you or anyone else to make
it so. However, returning invalid handle to a supposedly non-thread-safe
operation is just arrogance. A big fat warning (in the docs) saying that
QSqlDatabase and friends are not thread-safe should be enough. From then
on, it's my problem if I choose to cross that boundary. We don't want
nor need hand-holding from Qt developers.
":memory:?cache=shared"
To get it to access the same one. Does that help then?
No, because that means one in-memory database per process. I'll tell you
what has the potential to help, though:
SQLite has this relatively new feature called "URI Filenames" [1]. It
lets you have named in-memory databases[2] like so:
auto db = QSqlDatabase::addDatabase("QSQLITE",
   "file:foo?mode=memory&cache=shared");
This has two drawbacks:
1. What has been a simple incref is now a hash lookup + new connection
instantiation (and most probably some more stuff).
2. This requires the SQLITE_USE_URI=1 compile-time flag which is
disabled by default and also seems disabled in the embedded SQLite in
QtSql[3][5].
Not to mention a major refactoring effort from our side -- I think we
will need to visit every QSqlDatabase occurrence in our codebase to
restore pre-Qt-5.11 functionality.
Given all this, it is obvious to me that the right thing to do is to
revert 9b361f0e90fb1c154a16e65ec087ad36d5cca9b4[4]. I hope it is the
case for you as well.
Best regards,
Burak
[1]: https://sqlite.org/uri.html
[2]: https://sqlite.org/inmemorydb.html
[3]: https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/sqlite.pri
[4]:
https://code.qt.io/cgit/qt/qtbase.git/commit/src/sql/kernel/qsqldatabase.cpp?id=9b361f0e90fb1c154a16e65ec087ad36d5cca9b4
[5]: For reference, the following code:
int main() {
   QCoreApplication a(argc, argv);
   auto db = QSqlDatabase::addDatabase("QSQLITE", ":memory:");
   db.open();
   auto q = db.exec("pragma compile_options");
   while (q.next()) {
       qInfo() << q.value(0).toString();
   }
   return 0;
}
prints:
"COMPILER=msvc-1911"
"ENABLE_COLUMN_METADATA"
"ENABLE_FTS3"
"ENABLE_FTS3_PARENTHESIS"
"ENABLE_FTS5"
"ENABLE_RTREE"
"OMIT_COMPLETE"
"OMIT_LOAD_EXTENSION"
"THREADSAFE=1"
with the official Qt 5.11.1 MSVC 2017 64 bit.
Hi Burak,
I personally disagree, the warning was in the docs for some time and there were reports of people having issues because they did not see this documentation and they were having problems. I also think that it is only in the :memory: case where the change does hit a problem because there is no way for multiple connections to use it.
Turning on the SQLITE_USE_URI=1 define is worth considering at least to make it possible to use the URI filenames feature, I know it still has the first drawback but it is something at least. Currently however,I see no real need to revert the patch purely for this case. I am happy to open it up to see if anyone else has an opinion on this though.
Kind regards,
Andy
Fra: Interest <interest-bounces+andy.shaw=***@qt-project.org> på vegne av Burak Arslan <***@arskom.com.tr>
Dato: mandag 20. august 2018 16:25
Til: "***@qt-project.org" <***@qt-project.org>
Emne: Re: [Interest] QSqlDatabasePrivate::database: requested database does not belong to the calling thread.
Hey Andy,
Thanks for your reply.
On 08/20/18 10:21, Andy Shaw wrote:
Hi,
The QtSql classes themselves are not set to be reentrant or thread-safe (...) it may have worked by chance before but this was never guaranteed.
We the people can read the source code just as well as anyone and can already see that. However, it should be our decision to use the library the way we want.
I fully understand why making QSqlDatabase thread-safe may not be feasible and just to be clear, I'm not asking you or anyone else to make it so. However, returning invalid handle to a supposedly non-thread-safe operation is just arrogance. A big fat warning (in the docs) saying that QSqlDatabase and friends are not thread-safe should be enough. From then on, it's my problem if I choose to cross that boundary. We don't want nor need hand-holding from Qt developers.
Granted the ":memory:" case is a unique one in this regard because opening another QSqlDatabase on ":memory:" will give you a new one, however it seems you can do:
":memory:?cache=shared"
To get it to access the same one. Does that help then?
No, because that means one in-memory database per process. I'll tell you what has the potential to help, though:
SQLite has this relatively new feature called "URI Filenames" [1]. It lets you have named in-memory databases[2] like so:
auto db = QSqlDatabase::addDatabase("QSQLITE",
"file:foo?mode=memory&cache=shared"<file:///foo?mode=memory&cache=shared>);
This has two drawbacks:
1. What has been a simple incref is now a hash lookup + new connection instantiation (and most probably some more stuff).
2. This requires the SQLITE_USE_URI=1 compile-time flag which is disabled by default and also seems disabled in the embedded SQLite in QtSql[3][5].
Not to mention a major refactoring effort from our side -- I think we will need to visit every QSqlDatabase occurrence in our codebase to restore pre-Qt-5.11 functionality.
Given all this, it is obvious to me that the right thing to do is to revert 9b361f0e90fb1c154a16e65ec087ad36d5cca9b4[4]. I hope it is the case for you as well.
Best regards,
Burak
[1]: https://sqlite.org/uri.html
[2]: https://sqlite.org/inmemorydb.html
[3]: https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/sqlite.pri
[4]: https://code.qt.io/cgit/qt/qtbase.git/commit/src/sql/kernel/qsqldatabase.cpp?id=9b361f0e90fb1c154a16e65ec087ad36d5cca9b4
[5]: For reference, the following code:
int main() {
QCoreApplication a(argc, argv);
auto db = QSqlDatabase::addDatabase("QSQLITE", ":memory:");
db.open();
auto q = db.exec("pragma compile_options");
while (q.next()) {
qInfo() << q.value(0).toString();
}
return 0;
}
prints:
"COMPILER=msvc-1911"
"ENABLE_COLUMN_METADATA"
"ENABLE_FTS3"
"ENABLE_FTS3_PARENTHESIS"
"ENABLE_FTS5"
"ENABLE_RTREE"
"OMIT_COMPLETE"
"OMIT_LOAD_EXTENSION"
"THREADSAFE=1"
with the official Qt 5.11.1 MSVC 2017 64 bit.
