ad-freiburg / qlever-ui

A user interface for QLever
Apache License 2.0
19 stars 12 forks source link

Button to load more results has incorrect and misleading text #36

Open graue70 opened 2 years ago

graue70 commented 2 years ago
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.

tuukka commented 3 weeks ago

This behavior has changed but there are still issues with it.

To update on the original report, here's the current behavior:

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:

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])

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])).