fix(test): Remove non-matching search terms #915
Conversation
The Elasticsearch platform silently ignores search terms that don't match, as long as there are also enough matching terms. Thus, the search for 'document is a simple test' returns the license file although it doesn't contain the words "simple" or "test" or any variation thereof. Since the semantics of full text search aren't clearly defined, it should be up to the platform to decide how to handle non-matching search terms. So not returning the license in this case is just as valid! Case in point is PostgreSQL, which strictly requires every searched term (except for stopwords) to be contained in the search results (after normalization). These changes make the test harness pass on that platform, too. Signed-off-by: Jan-Philipp Litza <janphilipp@litza.de>
jplitza
force-pushed
the
fix-test-harness
branch
from
2cc8265 to
63396dd
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
Hello, Thanks for your work, but why would we care about search on postgresql here ? |
|
I do because I wrote fulltextsearch_sql. And the test harness of this (search platform independent) app is very useful for testing the platform implementation, too. So I tried to use it, and It's just this interpretation of additional words in the query that seems to differ between platforms. (MariaDB handles them the same way Elasticaearch does, it seems.) But if you say that the test suite is just supposed to be used with fulltextsesrch_elasticsearch in order to test this app (not the platform), feel free to close this PR. |
|
I am quite happy right now with the fact that some search content can be ignored, this is why you enforce the presence of a word in the result by prefixing with a '+' I really appreciate your work on this new extension, but can we keep the test as it is and have your parsing of the terms fit it ? |
2 participants
The Elasticsearch platform silently ignores search terms that don't match, as long as there are also enough matching terms.
Thus, the search for 'document is a simple test' returns the license file although it doesn't contain the words "simple" or "test" or any variation thereof.
Since the semantics of full text search aren't clearly defined, it should be up to the platform to decide how to handle non-matching search terms. So not returning the license in this case is just as valid!
Case in point is PostgreSQL, which strictly requires every searched term (except for stopwords) to be contained in the search results (after normalization). These changes make the test harness pass on that platform, too.