genenetwork / genenetwork2

GeneNetwork (2nd generation)
http://gn2.genenetwork.org/
GNU Affero General Public License v3.0
34 stars 24 forks source link

Fix fulltext search for certain searches #674

Closed zsloan closed 2 years ago

zsloan commented 2 years ago

Hyphens in fulltext searches were causing problems, but a recent commit I made to fix the issue (6496f75af521d6801c25de682d2852e14d597fed) apparently caused some issues for other types of searches, in addition to make such searches somewhat slower.

Apparently the issue wasn't just the hyphens, but also the text to either side of the hyphen being lower than the minimum word length (which is either 2 or 3 for us, can't remember). To try and address this, I did a regular expression check for a pattern with text of <3 legnth to either side of a hyphen, and when that's the case I add quotes from the search term plus an asterisk, which seems to be necessary to get it to not treat the hyphen as a delimiter and to correctly detect the search term during a boolean fulltext search.

arunisaac commented 2 years ago

Hyphens in fulltext searches were causing problems, but a recent commit I made to fix the issue (6496f75af521d6801c25de682d2852e14d597fed)

Could you also add a test case, if you haven't already, to ensure that we have no regressions on this issue?

Thanks!

zsloan commented 2 years ago

Yeah, I'm going to add one similar to the one here (that can check number of results) tomorrow - https://github.com/genenetwork/genenetwork2/blob/testing/test/requests/main_web_functionality.py

I'm not sure how to make one more specific than that (I'm sure it's possible, but I don't know how without doing something weird like comparing all the HTML), but that should at least be enough to verify that nothing has changed for one or two different searches relevant to this issue.

On Tue, Mar 1, 2022 at 12:02 AM Arun Isaac @.***> wrote:

Hyphens in fulltext searches were causing problems, but a recent commit I made to fix the issue (6496f75af521d6801c25de682d2852e14d597fed)

Could you also add a test case, if you haven't already, to ensure that we have no regressions on this issue?

Thanks!

— Reply to this email directly, view it on GitHub https://github.com/genenetwork/genenetwork2/pull/674#issuecomment-1055053858, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANQJGDFAZN4ZTALGJTB4UDU5WXIBANCNFSM5PSGWKBQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you modified the open/close state.Message ID: @.***>

BonfaceKilz commented 2 years ago

zsloan @.***> anaandika:

Yeah, I'm going to add one similar to the one here (that can check number of results) tomorrow - https://github.com/genenetwork/genenetwork2/blob/testing/test/requests/main_web_functionality.py

I'm not sure how to make one more specific than that (I'm sure it's possible, but I don't know how without doing something weird like comparing all the HTML), but that should at least be enough to verify that nothing has changed for one or two different searches relevant to this issue.

That's an E2E test though; and we'd have to resurrect Mechanical Rob.