Skip to content

Hangs when selecting all the records and deleting them #856

Closed
Closed
Hangs when selecting all the records and deleting them#856
Labels
bugConfirmed bugs or reports that are very likely to be bugs.osx
Milestone
3.10.0 - A Rubber Cactus Has Many Uses

Description

@revolter
revolter
opened on Nov 3, 2016

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:

Activity

  1. justinclift
    added
    bugConfirmed bugs or reports that are very likely to be bugs.
    osx
    on Nov 3, 2016

  2. prutz1311 commented on Nov 3, 2016

    @prutz1311
    prutz1311
    on Nov 3, 2016
    Contributor

    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?

  3. justinclift commented on Nov 3, 2016

    @justinclift
    justinclift
    on Nov 3, 2016
    Member

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

    Want to submit a PR? 😄

  4. revolter commented on Nov 4, 2016

    @revolter
    revolter
    on Nov 4, 2016
    MemberAuthor

    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.

  5. prutz1311 commented on Nov 4, 2016

    @prutz1311
    prutz1311
    on Nov 4, 2016
    Contributor

    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?

  6. justinclift commented on Nov 5, 2016

    @justinclift
    justinclift
    on Nov 5, 2016
    Member

    @MKleusberg What do you reckon?

  7. prutz1311
    added a commit that references this issue on Nov 16, 2016

    Fix hanging when deleting large number of records (sqlitebrowser#856)

    3265c84
  8. justinclift
    added a commit that references this issue on Nov 25, 2016

    Fix hanging when deleting large number of records (#856) (#870)

    17369b6

  9. revolter commented on Dec 21, 2016

    @revolter
    revolter
    on Dec 21, 2016
    MemberAuthor

    Can't we use the removeRows method instead?

  10. vtronko
    added a commit that references this issue on Dec 29, 2016

    Fix hanging when deleting large number of records (#856) (#870)

    5d0c1d9

  11. justinclift commented on May 20, 2017

    @justinclift
    justinclift
    on May 20, 2017
    Member

    @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. 😄

  12. justinclift
    added
    response requested
    on May 20, 2017
  13. justinclift
    added this to the 3.10.0 - A Rubber Cactus Has Many Uses milestone on May 20, 2017

  14. justinclift commented on Aug 20, 2017

    @justinclift
    justinclift
    on Aug 20, 2017
    Member

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

  15. justinclift commented on Nov 17, 2017

    @justinclift
    justinclift
    on Nov 17, 2017
    Member

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

  16. justinclift
    closed this as completedon Nov 17, 2017
  17. justinclift
    removed
    response requested
    on Nov 17, 2017

  18. revolter commented on Nov 17, 2017

    @revolter
    revolter
    on Nov 17, 2017
    MemberAuthor

    @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.

  19. justinclift commented on Nov 17, 2017

    @justinclift
    justinclift
    on Nov 17, 2017
    Member

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

  20. justinclift
    reopened this on Nov 17, 2017
  21. MKleusberg
    added a commit that references this issue on Nov 17, 2017

    Change label of "Delete record" button to reflect number of records

    5fbf5ca

  22. MKleusberg commented on Nov 17, 2017

    @MKleusberg
    MKleusberg
    on Nov 17, 2017
    Member

    Should be fixed 😃

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

  23. mgrojo commented on Jan 7, 2018

    @mgrojo
    mgrojo
    on Jan 7, 2018
    Member

    I suppose, this can be closed 😃

  24. justinclift commented on Jan 7, 2018

    @justinclift
    justinclift
    on Jan 7, 2018
    Member

    Good point. Thanks @mgrojo. 😄

  25. justinclift
    closed this as completedon Jan 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugConfirmed bugs or reports that are very likely to be bugs.osx

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions