Hangs when selecting all the records and deleting them

[revolter]

Details for the issue

When I select all the records in a big (~ 4000) table with cmd + a and hit the Delete Record (which should say Delete Records to be clear that it would delete multiple records) button, DB4S hangs for quite some time.

Firstly, if it really takes this long, it should show a progress bar.
Secondly, it would make sense to run DELETE FROM table if we notice that the user selected all the records, which would run very fast, instead of running one DELETE per record.
Also, wouldn't a transaction help?

Useful extra information

I'm opening this issue because:

• DB4S is crashing
• DB4S has a bug
• DB4S needs a feature
• DB4S has another problem

I'm using DB4S on:

• Windows: ( version: ___ )
• Linux: ( distro: ___ )
• Mac OS: ( version: ___ )
• Other: ___

I'm using DB4S version:

• 3.9.1
• 3.9.0
• Other: ___

I have also:

• Tried out the nightly version: https://github.com/sqlitebrowser/sqlitebrowser#windows
• Searched for an existing similar issue: https://github.com/sqlitebrowser/sqlitebrowser/issues?utf8=%E2%9C%93&q=is%3Aissue%20

[prutz1311]

What about using DELETE FROM tableWHERErowid IN ( ... ) here

sqlitebrowser/src/sqlitedb.cpp

Lines 873 to 876 in 60e4cb7

	QString statement = QString("DELETE FROM %1 WHERE %2='%3';")
	.arg(sqlb::escapeIdentifier(table))
	.arg(sqlb::escapeIdentifier(getObjectByName(table).table.rowidColumn()))
	.arg(rowid);

in all cases regardless of whether one or multiple records should be deleted?

[justinclift]

At first thought, yeah, that sounds like it would work. At least for rowid tables anyway.

Want to submit a PR? 😄

[revolter]

It looks like that method is called for each row here, so maybe this is where the IDs array should be created and passed to a new method that builds the correct query.

[prutz1311]

It turns out that the method removeRow of SqliteTableModel object gets invoked here, and since SqliteTableModel is a subclass of QAbstractTableModel which is, in turn, a subclass of QAbstractItemModel, the following code from Qt library is executed:

inline bool QAbstractItemModel::removeRow(int arow, const QModelIndex &aparent)
{ return removeRows(arow, 1, aparent); }

that is, it forces us to have only 1 row deleted.
My backtrace on deletion looks like this:

(gdb) backtrace
#0  DBBrowserDB::deleteRecord (this=0xba25c8, table=..., rowid=...)
    at /home/oleg/Документы/программирование/Sources/sqlitebrowser/src/sqlitedb.cpp:870
#1  0x00000000005ee5cf in SqliteTableModel::removeRows (this=0xb949d0, row=40, count=1, parent=...)
    at /home/oleg/Документы/программирование/Sources/sqlitebrowser/src/sqlitetablemodel.cpp:415
#2  0x00000000005a3a98 in QAbstractItemModel::removeRow (this=0xb949d0, arow=40, aparent=...)
    at /opt/qt57/include/QtCore/qabstractitemmodel.h:347
#3  0x000000000058fe72 in MainWindow::deleteRecord (this=0xba24a0)
    at /home/oleg/Документы/программирование/Sources/sqlitebrowser/src/MainWindow.cpp:589
#4  0x0000000000649bff in MainWindow::qt_static_metacall (_o=0xba24a0, _c=QMetaObject::InvokeMetaMethod, _id=17, _a=0x7fffffffd550)
    at /home/oleg/Документы/программирование/Sources/sqlitebrowser/build/moc_MainWindow.cpp:438
#5  0x00007ffff6cfc9d5 in QMetaObject::activate(QObject*, int, int, void**) () from /opt/qt57/lib/libQt5Core.so.5
#6  0x00007ffff7473f52 in QAbstractButton::clicked(bool) () from /opt/qt57/lib/libQt5Widgets.so.5
#7  0x00007ffff747414a in ?? () from /opt/qt57/lib/libQt5Widgets.so.5
#8  0x00007ffff7475648 in ?? () from /opt/qt57/lib/libQt5Widgets.so.5
#9  0x00007ffff74757c4 in QAbstractButton::mouseReleaseEvent(QMouseEvent*) () from /opt/qt57/lib/libQt5Widgets.so.5
#10 0x00007ffff73d85da in QWidget::event(QEvent*) () from /opt/qt57/lib/libQt5Widgets.so.5
#11 0x00007ffff7394cbc in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /opt/qt57/lib/libQt5Widgets.so.5
#12 0x00007ffff739c61b in QApplication::notify(QObject*, QEvent*) () from /opt/qt57/lib/libQt5Widgets.so.5
#13 0x00007ffff6cd48e5 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /opt/qt57/lib/libQt5Core.so.5
#14 0x00007ffff739b0a3 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool)
    () from /opt/qt57/lib/libQt5Widgets.so.5
#15 0x00007ffff73f14e1 in ?? () from /opt/qt57/lib/libQt5Widgets.so.5
#16 0x00007ffff73f3933 in ?? () from /opt/qt57/lib/libQt5Widgets.so.5
#17 0x00007ffff7394cbc in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /opt/qt57/lib/libQt5Widgets.so.5
#18 0x00007ffff739bc40 in QApplication::notify(QObject*, QEvent*) () from /opt/qt57/lib/libQt5Widgets.so.5
#19 0x00007ffff6cd48e5 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /opt/qt57/lib/libQt5Core.so.5
#20 0x00007ffff798319b in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) ()
   from /opt/qt57/lib/libQt5Gui.so.5
#21 0x00007ffff7984cf5 in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) ()
   from /opt/qt57/lib/libQt5Gui.so.5
#22 0x00007ffff7966dab in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /opt/qt57/lib/libQt5Gui.so.5
#23 0x00007fffef8b13f0 in ?? () from /opt/qt57/plugins/platforms/../../lib/libQt5XcbQpa.so.5
#24 0x00007ffff4755e04 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#25 0x00007ffff4756048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#26 0x00007ffff47560ec in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#27 0x00007ffff6d2329c in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /opt/qt57/lib/libQt5Core.so.5
#28 0x00007ffff6cd2bbb in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /opt/qt57/lib/libQt5Core.so.5
#29 0x00007ffff6cda9b6 in QCoreApplication::exec() () from /opt/qt57/lib/libQt5Core.so.5
#30 0x0000000000634f58 in main (argc=2, argv=0x7fffffffe3b8)
    at /home/oleg/Документы/программирование/Sources/sqlitebrowser/src/main.cpp:13
(gdb) 

It seems to me that we cannot delete multiple records at once if we rely on table model of Qt.
Should we bypass the inline bool QAbstractItemModel::removeRow method call?

[justinclift]

@MKleusberg What do you reckon?

[revolter]

Can't we use the removeRows method instead?

[justinclift]

@revolter Would you have time to see if this is working decently in the nightlies? @innermous updated the deletion code to use removeRows, so in theory it should be ok now. 😄

[justinclift]

@revolter Ping. But, check with 3.10.0 release instead. 😄

[justinclift]

Closing this as it seems to have been fixed ages ago. 😄

[revolter]

@justinclift, Sorry about that, I'm way way behind with the notifications. It does delete a lot of records in an instant, though I still think it would make sense for the delete button's text to change to Delete records as soon as you select more than one record.

[justinclift]

Ahhh, that's a good point. Yeah, that would be a small bit of nicety we could add. 😄

[MKleusberg]

Should be fixed 😃

@revolter Same for me, I've totally lost track of the open issues and notifications lately 😉

[mgrojo]

I suppose, this can be closed 😃

[justinclift]

Good point. Thanks @mgrojo. 😄