Button to load more results has incorrect and misleading text
graue70 opened this issue · 1 comments
PREFIX wdt: <http://www.wikidata.org/prop/direct/>
SELECT DISTINCT ?item ?type WHERE {
?item wdt:P279 ?type .
}
LIMIT 1000
If you execute the above query, the button on top of the results table says Limited to 40 results. Show all 3,018,709 results.
. If you click on it, the results table does NOT show 3M results, but shows 1000 results. This is confusing.
The button text should tell me what will really happen if I click on it. In this case, it should recognize the LIMIT
in the query and say Show 1000 results
.
It might even make sense to implement a two-step process: The table shows 40 results. The button says Show 1000 results
. I click on the button to show the 1000 results I specified in the query. Now the button says Show all 3,018,709 results
. If I click on it, all 3M rows are shown.
This behavior has changed but there are still issues with it.
To update on the original report, here's the current behavior:
-
One button says "1,000 lines found" - is that as expected? The query requested only 1,000 to be found although there are more results that QLever can/could find.
-
Another button says "Limited to 100 results; show all 1,000 results" - this is now correct, although there could be a clarification "up to LIMIT". (Perhaps the first button should speak about "results" as well)?
-
When I click the button, the table reloads to show all the 1,000 results as expected. Unexpectedly, the button disappears. It could switch to saying "Showing all 1,000 results up to LIMIT." Alternatively, it could say "Remove the LIMIT" (if it can change the query like that).
Problematic test case
Take a query that has a LIMIT
higher than the number of results that could be found, for example:
PREFIX wdt: <http://www.wikidata.org/prop/direct/>
PREFIX wd: <http://www.wikidata.org/entity/>
SELECT DISTINCT ?item ?type WHERE {
?item wdt:P279 wd:Q11002 .
}
LIMIT 1000
After executing this query, the table shows all the 7 results. However:
-
The first button says "1,000 lines found" which cannot be true. I would expect it to say "7 lines/results found"
-
The second button says "Limited to 7 results; show all 1,000 results" while the results were not limited and 1,000 results are not available. I would expect it to be disabled and say "Showing all 7 results."
-
Clicking the button reloads the results but (as one might guess) nothing changes.
Logic
The UI code has this logic:
let limitMatch = result.query.match(/\bLIMIT\s+(\d+)\s*$/);
if (limitMatch) { resultSize = parseInt(limitMatch[1]); }
(The regex causes confusion e.g. if the LIMIT
is in a sub-query or you comment out a LIMIT
line but the regex still matches, or if you add a comment to the end of the line and the regex doesn't match anymore.)
Before these lines, resultSize
is what QLever says i.e. how many results would be available without any limits.
The current UI logic clearly doesn't work when there's a LIMIT
and less results are available than the limit.
The inputs we have:
variable | meaning | comments |
---|---|---|
parseInt(limitMatch[1]) |
the parsed LIMIT if any |
would be more robust to get this from the backend |
sendLimit |
number of results that the UI asked from the backend | 0 means unlimited |
result.resultSize |
number of results that would be available without limits | |
nofRows |
number of rows returned from the backend |
Invariant: nofRows <= result.resultSize
Invariant when sendLimit
is greater than 0: nofRows <= sendLimit
Invariant when there's a LIMIT
: nofRows <= parseInt(limitMatch[1])
-
If
nofRows
is less thansendLimit
(orsendLimit
is 0), the only way to show more results is to remove aLIMIT
(if there is one and it's equal tonofRows
andresult.resultSize
is larger). No need for the second button. -
If
nofRows
is equal tosendLimit
, it might be because ofsendLimit
, because ofLIMIT
if equal, or becauseresult.resultSize
is equal. Ifresult.resultSize
is greater thannofRows
and there is no equalLIMIT
, show the second button for removing thesendLimit
. -
(
nofRows
should never be greater thansendLimit
whensendLimit
is not 0).
The current condition to show the second button:
if (nofRows < parseInt(resultSize)) {
I think the condition should instead be if (nofRows < result.resultSize && nofRows !== parseInt(limitMatch[1]))
.