Skip to content
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

Search method returns undefined elements #29

Closed
mateonunez opened this issue Jul 18, 2022 · 3 comments · Fixed by #33
Closed

Search method returns undefined elements #29

mateonunez opened this issue Jul 18, 2022 · 3 comments · Fixed by #33
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@mateonunez
Copy link
Collaborator

mateonunez commented Jul 18, 2022

Describe the bug
Searching by a term that doesn't exist, Lyra returns X undefined elements.

The error also occurs using limit, property or exact params.

To Reproduce
Steps to reproduce the behavior:

  1. lyra.search({ term: "whatever" })

Expected behavior
Lyra shouldn't return X undefined elements. Instead of that should return an empty array.

Screenshots

image

Desktop:

  • OS: Windows 11
  • Environment: Node
  • Node: v18.6.0
  • pnpm: v7.5.1
  • Lyra: v0.0.1-beta-12
@micheleriva micheleriva added the bug Something isn't working label Jul 19, 2022
@micheleriva
Copy link
Member

micheleriva commented Jul 19, 2022

When retrieving data, we create an array of x length, where x is the limit search param. That way we avoid unnecessary dynamic allocations when adding data to the results array, but of course is causing this problem

const results: RetrievedDoc<TSchema>[] = Array.from({
  length: limit,
});

Thanks for opening the issue!

@micheleriva micheleriva added the good first issue Good for newcomers label Jul 19, 2022
@mateonunez
Copy link
Collaborator Author

mateonunez commented Jul 19, 2022

And what would be the correct way to fix it?
Create an Array that also allows dynamic allocations (in my opinion could be fixed in this way)? Or filter the Array before the return?

@micheleriva
Copy link
Member

I'd say... let's benchmark these operations with different array sizes. Let's see when is convenient filtering vs dynamic arrays given the docs we have

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants