Skip to content

Conversation

@sandman7920
Copy link
Contributor

Workaround for #2592 and #2787

This was implemented as a workaround for #2592 and #2787.
There is an issue with QTableView integer overflow (int32_t), when
QTableView::defaultRowSize() = 30, maximum number of rows which can
be rendered is 0x7fffffff / 30 = 71,582,788. When a huge SQLite table is
selected in the BrowserTab, DB4S hangs in an infinite render loop.

More info #2592 (comment)
The value is not visible in the Preferences dialog, but can be overwritten
from the command line.
@MKleusberg
Copy link
Member

Thank you very much for this, @sandman7920! I haven't had the chance to review this in detail yet. But two general thoughts from my side:

  1. Ideally we would override the vertical scrollbar behaviour and only show a window of e.g. 1,000,000 rows at a time and then move that window when using the scrollbar. This way the user could see all rows. The downside is that this makes the code even more complex than it already is and that it would somewhat break the search function (Ctrl+F). So in my opinion your solution is what we should aim for at the moment.
  2. 👍 for not adding this to the Preferences dialog. But I was wondering whether we could even calculate the maximum number of rows like from the size of an int and the default (or the actual?) row height to show as many rows as possible. Not sure if it's worth the effort though.

@sandman7920
Copy link
Contributor Author

But I was wondering whether we could even calculate the maximum number of rows like from the size of an int and the default (or the actual?) row height to show as many rows as possible

Yes, we could, but if the user resizes only one row, we are in the same position.

@sandman7920
Copy link
Contributor Author

Another possible trigger for this issue is:
image

Maybe we should do something like:

tableView.verticalHeader()->setMaximumSectionSize(0x7fffffff / rows_limit);

@mgrojo
Copy link
Member

mgrojo commented Aug 16, 2021

Yes, something like that makes sense too.

@MKleusberg MKleusberg force-pushed the master branch 5 times, most recently from d396227 to 16035be Compare December 26, 2021 13:41
Copy link
Member

@mgrojo mgrojo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider this ready to merge. I'll do it in some days, if there are no objections.

@lucydodo lucydodo force-pushed the master branch 3 times, most recently from 4bc0c36 to e0a000c Compare July 2, 2023 13:12
@mgrojo
Copy link
Member

mgrojo commented Sep 6, 2023

Let's merge this. Nobody objected after more days than I initially thought were going to pass. 😄 Sorry for the long delay.

@mgrojo mgrojo merged commit 4e43233 into sqlitebrowser:master Sep 6, 2023
@mgrojo mgrojo mentioned this pull request Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants