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

Fix bug suggestions with PostgreSQL #7988

Merged
merged 7 commits into from
Mar 26, 2024

Conversation

vrigal
Copy link
Collaborator

@vrigal vrigal commented Mar 22, 2024

Closes #7986

This PR adds a unit test that reproduce the bug suggestion described by @Archaeopteryx in #7986 that appeared when switching prototype database to Postgresql.

It turns out we cannot rely on Postgresql Full-Text search capabilities for this use-case, as we are trying to mostly match filenames or paths with special characters like slashes or underscores, but postgresql search functions are optimized for tokenization with specific languages instructions (basically we do not search for english...).

So we had to reproduce the Mysql implementation:

  1. using pattern matching with ILIKE clause
  2. then trigram similarity to get a ranking amongst multiple bugs

⚠️ This requires a specific Postgresql extension to run (available by default)

@vrigal
Copy link
Collaborator Author

vrigal commented Mar 22, 2024

@Archaeopteryx Hello.
This is an attempt to reproduce the bug where no bug is fetched with PostgreSQL, described in #7986.
It is not finished yet (it seems the test passes with postgres actually), I will continue working on this on monday.

@vrigal vrigal changed the title WIP: Reproduce #7986 Fix bug suggestions with PostgreSQL Mar 25, 2024
@La0 La0 marked this pull request as ready for review March 25, 2024 19:48
@La0 La0 requested a review from Archaeopteryx March 25, 2024 19:48
Copy link
Collaborator

@Archaeopteryx Archaeopteryx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fast fix.

There are some cases which still don't get a suggestion despite the search with a test name and the bug in the database. Example 1 and Example 2.
Could you take another look?
The affected tests all have a - in their name, as far as I can see.

@vrigal
Copy link
Collaborator Author

vrigal commented Mar 26, 2024

@Archaeopteryx I looked as your examples, and updated the test with that bug suggestion match from treeherder.mozilla.org.
Unfortunately, I could not reproduce (the test passes).
I'm actually working on updating tests to make ordering possible by -similarity.

@codecov-commenter
Copy link

codecov-commenter commented Mar 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.08%. Comparing base (d8e7f0d) to head (3b40e92).
Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7988      +/-   ##
==========================================
+ Coverage   77.06%   77.08%   +0.01%     
==========================================
  Files         544      545       +1     
  Lines       26940    26958      +18     
  Branches     3382     3382              
==========================================
+ Hits        20762    20780      +18     
  Misses       6011     6011              
  Partials      167      167              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vrigal
Copy link
Collaborator Author

vrigal commented Mar 26, 2024

-similarity was due to a minor scoring difference between MySQL and Postgres. I updated the fixture and it looks fine now.
I have no hint about the missing suggestions for now.

@Archaeopteryx Archaeopteryx merged commit 931e8bb into mozilla:master Mar 26, 2024
5 checks passed
Archaeopteryx added a commit to Archaeopteryx/treeherder that referenced this pull request Apr 26, 2024
* Use trigram similarity instead of FTS

* Skip escaping special characters

* Order results by match ranking on Postgres
---------

Co-authored-by: Bastien Abadie <[email protected]>
Co-authored-by: Sebastian Hengst <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants