Feature #15429
closed[Workbench] Use trigram based full text search.
Description
Since we plan on deprecating @@, as an available API list method filter, we should switch Workbench 1 full text search to use the trigram index based ["any", "ilike", "%%"] filter.
Updated by Eric Biagiotti over 5 years ago
- Related to Idea #15430: [API] Remove the @@ list method filter added
Updated by Tom Morris over 5 years ago
- Target version changed from To Be Groomed to Arvados Future Sprints
Updated by Tom Morris over 5 years ago
- Target version changed from Arvados Future Sprints to 2019-09-25 Sprint
Updated by Lucas Di Pentima over 5 years ago
- Assigned To set to Lucas Di Pentima
Updated by Lucas Di Pentima about 5 years ago
- Target version changed from 2019-09-25 Sprint to 2019-10-09 Sprint
Updated by Tom Morris about 5 years ago
- Target version changed from 2019-10-09 Sprint to 2019-10-23 Sprint
Updated by Lucas Di Pentima about 5 years ago
- Status changed from New to In Progress
Updated by Lucas Di Pentima about 5 years ago
Updates at b93675913 - branch 15429-wb1-use-trigram-search
Test run: https://ci.curoverse.com/job/developer-run-tests/1590/
- Replaced
to_tsquery()
withto_tsquery_filters()
that adds a["any", "ilike", "%term%"]
for every "word" on the user search query. - No test updates needed as integration tests don't care how the search is done.
Some clarification needed: On to_tsquery()
's comment there were some cases documented stating that underscore chars should be treated as non-word, but from my manual tests it seems that isn't the case with the current regular expression. Not sure if the comments were old or the code buggy and I should add that feature to the new function.
Updated by Tom Clegg about 5 years ago
Since it's no longer doing tsquery things, to_tsquery_filters() doesn't seem like the best name -- maybe ilike_filters()?
""." is a word char in FT queries" → "...ilike queries"
comment "It returns null if..." → "It returns [] if..." (good change!)
to_tsquery_filters(q).forEach(function(f) { filters.push(f) }) ...could be just filters = filters.concat(to_tsquery_filters(q)) like the line above
Updated by Lucas Di Pentima about 5 years ago
Updates at 13ac0cadc
Test run: https://ci.curoverse.com/job/developer-run-tests/1593/
Addressed the above comments and also renamed the to_tsquery.js
file to ilike_filters.js
Updated by Lucas Di Pentima about 5 years ago
Retrying integration tests at https://ci.curoverse.com/job/developer-run-tests-apps-workbench-integration/1686/
Updated by Tom Clegg about 5 years ago
Lucas Di Pentima wrote:
Some clarification needed: On
to_tsquery()
's comment there were some cases documented stating that underscore chars should be treated as non-word, but from my manual tests it seems that isn't the case with the current regular expression. Not sure if the comments were old or the code buggy and I should add that feature to the new function.
I don't think it matters: the purpose of defining "word chars" in the previous code was to match the postgres full-text query behavior. The new code just has to behave sensibly with "ilike". I suppose we can either treat an underscore as a word char and leave it alone (so it matches any single char) or escape it with a backslash (so it matches a literal underscore). I'd guess that most users will expect it to match a literal underscore, but the wildcard option also seems reasonable.
Updated by Lucas Di Pentima about 5 years ago
Escapes the "_
" char so it can be used as a literal at a238ab719
Test run: https://ci.curoverse.com/job/developer-run-tests/1595/
Integration tests retry: https://ci.curoverse.com/job/developer-run-tests-apps-workbench-integration/1690/
Updated by Lucas Di Pentima about 5 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|beb4aebe0492f65e401cbaf640b677a6ce42f947.
Updated by Lucas Di Pentima almost 5 years ago
- Related to Idea #15333: Workbench2 feature parity with Workbench added