-
Notifications
You must be signed in to change notification settings - Fork 212
Improve search functions #721
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of comments! This already looks way better than the existing search though, thank you for tackling this :)
I would like to see a lot more tests added before this is merged - the search is now pretty complicated and I want to make sure this behaves as expected. Some things to test
- exact matches should always appear first
- crates that never had a successful build should not be shown
- no more than
LIMIT
crates are returned - searching crate descriptions works
- searching
regex
shows results forregex-syntax
- searching
Regex
countsregex
as an exact match- maybe we should just turn the query into all lowercase to start?
- searching 'redex' shows results for
regex
(fuzzy searching)
#708 brings up a good point, does the actual date a crate was published matter when we have semver? |
I don't know how to sort by semver in SQL. I think we should wait on that until later, we're making a big change already. |
Looks like you need to remove the migration to get the tests to pass: https://github.com/rust-lang/docs.rs/pull/721/checks?check_run_id=606111571#step:14:80 |
You need to rebase over the changes to CI. Additionally, you may need to write an extra (small) dockerfile to add the fuzzy-search extension to postgres, see https://stackoverflow.com/a/54630526/7669110 for an example of what that would look like. Past that, the current blocker is the performance of the query. The old query took less than half a second, this one takes almost 3. We've been working on this in Discord. |
Got a much-improved query in, had to remove description searching, as with it the query took 700-800ms, while without it the query takes a consistent 300ms |
…pi and added tests
Co-Authored-By: Joshua Nelson <[email protected]>
🎉 🎉 🎉 |
Closes #489, #156 and #13
serialization
will also pull up results forserde
(With any better name matches before it)