-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Confusing nbHits #1120
Comments
Well, I don't really see the nuance between nbHits and nbMatches to be honest... I think that what makes it confusing is that it is linked to exhaustiveNbHits which is probably overlooked. I guess |
What I understand since I use Meili and read the docs:
=> You can have 2 hits in the array
That could be a good start! |
Another user who seems to be confused with |
I will write an article about pagination, because there actually is a workaround now that we have #849. That does not change the necessity to address the naming issue. @Kerollmops proposed that we drop the |
I like the naming, but most of all, before even renaming it: what would be the usecase of |
this is the PR that introduced the feature: #541 For me it is used to report number of results, and for pagination, as long as you don't need to access the last page/random page. This last reason was a known limitation when implementing the first iteration of the feature. |
Even to build a "non-finished" pagination, I mean with no page number, Am I wrong @bidoubiwa? Can you confirm what I say about pagination and |
@curquiza I am not sure to understand why that would not be possible? What do we decide about the renaming? |
I have the feeling there are two main problems raised here: 1. Matches and Hits are confusing terms used for different purposes arbitrarilyMatchesin the search request parameters: {
"matches": Boolean // if true, indexes are provided for the croped content
} This is in my opinion, not an intuitive naming. But mostly, because it uses matches, it would be confusing to use it anywhere else in the search route. In the documentation, HitsIn the search response : hits: array // List of returned documents by MeiliSearch (ex: `20`)
nbHits: number // List of "matching" documents inside MeiliSearch (ex: `1229`)
exhaustiveNbHits: boolean // If the "matching" documents inside MeiliSearch is exhaustive or not In the same returned object by the search route, For example: 2. Pagination is not recommended
The user would have to add an additional documents to the limit parameter to ensure using the hits length that there is an additional document that can be loaded in the next "page" or "scroll". Since Another problem. but I'm not sure this is linked to this issue, is that how greater our offset becomes, how more MeiliSearch will have to bucket-sort to find - for example - the In either case, because it does not serve our main objective of I'm sorry if I missed some subtleties, which I'm sure I did. ExhaustiveNbHits NamingIf we decide to keep it, I think, following what i previously said on hits and matches, and @Kerollmops suggestion, I would like it to be called ConclusionI don't understand the purpose of |
80: Fix getTotalCount() method r=curquiza a=curquiza Remove `nbHits` usage because this is not reliable information. The Meili team is aware of this. Here are the different issues and comments about it to explain why it's confusing, and why we should not use it: - meilisearch/meilisearch#1120 - meilisearch/documentation#561 - meilisearch/meilisearch-php#119 (comment) TLDR; `nbHits` is not reliable for pagination because can be exhaustive or not, depending on the value of `exhaustiveNbHits` that MeiliSearch returns which is always `false` for the moment. We are sorry for this. We all hope this confusion will be fixed asap in MeiliSearch.⚠️ The linter error in the CI will be fixed with #82 Co-authored-by: Clémentine Urquizar <[email protected]>
I'm closing this one since the main problem comes from the pagination. We have specified a finite pagination https://github.com/meilisearch/specifications/pull/42/files Concerning the naming we will pass on it during the stabilization phase of the different API endpoints before 1.0 |
I open an issue before opening any spec to be sure I understand well the topic.
It’s been many times I notice the users are confused with the role of
nbHits
.If I understand well
nbHits
is the number of matches. See the docs about the definition ofnbHits
: https://docs.meilisearch.com/references/search.html#responseAnd most of all,
nbHits
is only an estimation (until we release the feature that allows the exhaustivenbHits
) and cannot be reliable for accuracy.Problems
The problems are:
nbHits
for pagination, but they should not. See the description of this issue, where the user explains his/her context: Total result count (nbHits) not updated for filters #764. Or see this issue and my comment: Custom pagination with totalCount meilisearch-laravel-scout#60 (comment)nbHits
to know the number ofhits
, I mean the length of thehits
array. Which is fair, the wordhits
is used in both fields. See my comment on this issue: Improve the search() method meilisearch-php#119 (comment)Solutions
To solve these issues, at least the second point, I would suggest renaming
nbHits
into something else which is nothits
, likenbMatches
. Indeed,nbHits
is not the number of hits but the number of matches, if I understand well again.You can ask for a
limit
of 2, so the total number of hits returned by MeilISearch is 2. But also, you have 23 matches for your query. With this example, thenbHits
would be set to 23 despite the length of the hits array would be 2. Am I right?My question is: would this be correct to change the name of
nbHits
? Or do I miss understand something in the notion of « number of hits »? The naming will be discussed in the spec/meeting of course.To prevent the first point, the documentation could warn and guide the user through a good pagination set up. See the issue Charlotte has already created: meilisearch/documentation#561
We could also think about renaming
hits
, but again, we will discuss in a spec if the change is possible.Edit
The kind of change I have to do because of contributors confusion: meilisearch/meilisearch-laravel-scout#80
The text was updated successfully, but these errors were encountered: