Selecting a table (browse data) results in 4 queries

chrisjlocke commented 7 years ago

Details for the issue

When a table is selected in the 'Browse Data' tab, four queries are run:

SELECT COUNT(*) FROM (SELECT '_rowid_',* FROM 'table_name' ORDER BY '_rowid_' ASC); SELECT '_rowid_',* FROM 'table_name' ORDER BY '_rowid_' ASC LIMIT 0, 10000; SELECT COUNT(*) FROM (SELECT '_rowid_',* FROM 'table_name' ORDER BY '_rowid_' DESC); SELECT '_rowid_',* FROM 'table_name' ORDER BY '_rowid_' DESC LIMIT 0, 10000;

Is there a reason why data is fetched twice? On larger tables, this is quite a delay. Selecting a count shouldn't need to order by either - again, isn't that forcing the database to do something unnecessarily - a table with a million rows - 'can you please sort this, as I need to know how many rows there are....' ... 'oh, and can you do that again please, as I now need to display this data'.,,.*

(*Ignoring the fact that there are indexes, etc, etc.)

Useful extra information

I'm opening this issue because:

[ ] DB4S is crashing
[ ] DB4S has a bug
[x] DB4S is a bit weird
[ ] DB4S needs a feature
[ ] DB4S has another problem

I'm using DB4S on:

[x] Windows: ( version: ___ )
[ ] Linux: ( distro: ___ )
[ ] Mac OS: ( version: ___ )
[ ] Other: ___

I'm using DB4S version:

[ ] 3.9.1
[ ] 3.9.0
[x] Other: _Latest nightly__

I have also:

[x] 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

MKleusberg commented 7 years ago

Not sure why you get 4 queries, two for ASC and two for DESC. Have you changed the sort order in between?

You're absolutely right about the sorting & counting thing. I have just done a quick test and this

explain select count(*) from (select rowid,* from table_name)

generates 9 rows for my test table (with PK), while this

explain select count(*) from (select rowid,* from table_name order by rowid desc)

generates 40 rows. More instructions don't necessarily mean longer execution times but it probably does in this case.

We have to fetch the data twice (the count and separately the data) because of the scrolling. We need to know the number of rows to set the scrollbar maximum but SQLite can't tell us the number of rows before having sent the last one. So we do the count as a workaround. This is the same as in issue #409.

justinclift commented 7 years ago

As a thought, is the "ORDER BY" in the query added regardless, or is that just due to the approach @chrisjlocke was using for the cut-n-pasted example??

MKleusberg commented 7 years ago

It depends. For the COUNT queries we just take the original SELECT statement as-is and insert it here:

SELECT COUNT(*) FROM (original_statement);

So, if the original statement contains an ORDER BY clause it'll be used in the count query, too. And if not, we won't insert one.

What we would need to do is remove the ORDER BY clause (if there is one) from the original statement. It's a bit tricky because we don't have a parser for select statements yet but can be done with a regex.

chrisjlocke commented 7 years ago

or is that just due to the approach @chrisjlocke was using for the cut-n-pasted example??

A table was simply selected from the drop-down box in the 'Browse Data' tab ....

chrisjlocke commented 7 years ago

Have you changed the sort order in between?

No - a table was selected from the drop-down box in the 'Browse Data' tab,,,

MKleusberg commented 7 years ago

@chrisjlocke Got it! If I change the default sort order, switch the table, and then come back to the first table, I get 4 queries, too. That definitely need to be addressed, too.

chrisjlocke commented 7 years ago

If I change the default sort order, switch the table, and then come back to the first table...

Interesting, as on mine I haven't set a sort order, so simply switching tables...

MKleusberg commented 7 years ago

Hmm, to be honest I have no idea how that happens because the default sort order is ascending while your query sorts descending (well, in the second try, that is). I assume you have already tried restarting the application?

MKleusberg commented 7 years ago

Let me prepare a quick list of ToDo items for this:

[x] Don't execute four queries where two are sufficient. No need to first query the data in the one sorting order and then again in the other order.
[x] Remove the ORDER BY clause (if there is one) from the query when doing a count.
[ ] Ideally, get rid of the count query altogether. But this might break the scrolling.

justinclift commented 7 years ago

Hmmm, if we need the count(), can we quietly add it into the original query so it all gets done as part of a single statement?

MKleusberg commented 7 years ago

@justinclift I don't think so. Or at least I don't know how the statement should look like. Do you have something in mind there?

@chrisjlocke Can you test again using the latest version from master? In theory, the first todo item should be implemented in that version :smiley:

justinclift commented 7 years ago

@MKleusberg Hmmm, upon thinking about it more... nah, the thought I had in mind was way too simplistic to work. Doh. :wink:

MKleusberg commented 7 years ago

@justinclift No worries :laughing:

@chrisjlocke The descending sort order you get seems to be the same issue as #641. Any idea how to track that down? Does it happen for all databases/tables for you?

chrisjlocke commented 7 years ago

I was going to say 'yes, across all databases and tables', but I'm not so sure. By clicking across the tabs, it did create a rogue ASC query.

Also, after opening another database, its now just doing two queries, not four.
Will test further...

chrisjlocke commented 7 years ago

Hmm. After a bit of head scratching, it appears the first time you open a database, browsing a table is ASC. After that, its DESC.

http://screencast-o-matic.com/watch/cbhiQLXZGE

MKleusberg commented 7 years ago

The video helps a lot. Thanks for that! But I still have no idea how that can happen :wink: Are you compiling DB4S by yourself or are you using the nightlies? I'm asking because we might be able to narrow down the source of the problem by adding some debug outputs here and there.

chrisjlocke commented 7 years ago

Using the Windows nightlies. 💨

MKleusberg commented 7 years ago

Can you test tomorrow's nightly build for this issue, too (as well as for #1019)? The 4 queries problem might be fixed by that as well :smiley:

chrisjlocke commented 7 years ago

No problem. Thanks.

chrisjlocke commented 7 years ago

Well, a 1% improvement.

SELECT COUNT(*) FROM (SELECT `_rowid_`,* FROM `customers` ORDER BY `_rowid_` ASC);
SELECT `_rowid_`,* FROM `customers` ORDER BY `_rowid_` ASC LIMIT 0, 10000;
SELECT COUNT(*) FROM (SELECT `_rowid_`,* FROM `customers` ORDER BY `_rowid_` ASC);
SELECT `_rowid_`,* FROM `customers` ORDER BY `_rowid_` ASC LIMIT 0, 10000;
SELECT COUNT(*) FROM (SELECT `_rowid_`,* FROM `invoices` ORDER BY `_rowid_` ASC);
SELECT `_rowid_`,* FROM `invoices` ORDER BY `_rowid_` ASC LIMIT 0, 10000;
SELECT COUNT(*) FROM (SELECT `_rowid_`,* FROM `invoices` ORDER BY `_rowid_` ASC);
SELECT `_rowid_`,* FROM `invoices` ORDER BY `_rowid_` ASC LIMIT 0, 10000;

Still getting two queries each time I browse a table, but the sort order is maintained.... ;) (Two tables were browsed, resulting in four sets of queries)

justinclift commented 7 years ago

Just took a quick run at this again...

This seems to merge the two SQL statements into a single one properly:

WITH cnt AS (
    SELECT count(*) AS row_count
    FROM `customers`
)
SELECT db.`_rowid_`, db.*, cnt.row_count
FROM `customers` AS db, cnt;

That'll append a new column "row_count" to the end of the returned list, with the count(*) result in it. It's the same value in every row, so just read any of them.

In theory, the optimiser in SQLite should be smart enough to not scan through the table rows twice when this is run... but I don't know for sure.

It'd be possible to figure out with some usage of EXPLAIN + real world time measurements using a larger sized database:

https://dev1.dbhub.io/datasciesotist/DMOZ_category.db

:wink:

Hmmm, this does similar:

SELECT db.`_rowid_`, db.*, (SELECT count(*) FROM `customers`)
FROM `customers` AS db;

Meh, I'm getting distracted. Going to leave this alone now and jump back on the Golang stuff instead. :smile:

mgrojo commented 6 years ago

I've edited the list of @MKleusberg above. The first bullet and the reason why this issue was open at first time should already be fixed by c78c03bf0b040b58977ba1ebbd284ea69531b939 but the other points still could be improved.

mgrojo commented 3 years ago

We can finally consider this issue fixed!

sqlitebrowser / sqlitebrowser