OCA
[16.0][IMP] base_search_mail_content: Use the GIN index correctly
The purpose of the GIN index is for full-text search, but without specifying the operator, Odoo does not use the index and instead searches using LIKE. This commit explicitly adds the % operator.
TT51245
@Tecnativa @pedrobaeza could you please review this?
Hi @yostashiro Sorry, but this change is out of scope. If I understand you correctly, you expect Odoo to search using each word entered individually. In my opinion, this is not correct.
Please note that this module uses a GIN index. My intention is only to enable the search to correctly use that index, which currently doesn't happen because it's using the LIKE operator instead of the % (full-text search) operator.
Once the module uses the correct operator, the GIN index will support full-text search. For example, if a message contains "Hello World", you’ll be able to search using "world hell", see the test case for reference.
Note: Please attach a screenshot with the message in English. I couldn't fully understand what it says.
@carlos-lopez-tecnativa I understand your intention. It's just that pg_trgm doesn't seem to work well with CJK characters, so your implementation will make the module unusable for the users in our market.
For example, if a message contains "Hello World", you’ll be able to search using "world hell", see the test case for reference.
The code I suggest should function the same way, but I guess it's not as performant as using a GIN index.
I think it would be nice to make the search method configurable. Would you consider doing it in this PR? If not, we could create a PR to take care of that.
Note: Please attach a screenshot with the message in English. I couldn't fully understand what it says.
I don't think you need to understand what it says. I was just showcasing that the search didn't work with Japanese characters. :)
Yeah, pg_trm doesn't work with Japanese characters, but having that this module depends on base_search_fuzzy, which bases all its feature in trigram indexes, isn't better to just create a new module mail_search_content_by_word or similar with just that specific code? Note that there is base_name_search_improved that does something similar.
@pedrobaeza Thanks, we could go as you suggest. However, IMO, the truly optimal design would be to remove the dependency on base_search_fuzzy from base_search_mail_content, and instead create a separate module such as mail_search_content_trgm which depends on base_search_fuzzy and base_search_mail_content. If we go with creating mail_search_content_by_word, the majority of the code will be a copy of base_search_mail_content.
Well, I'm not sure there's so much code to share between both, but in any case, that should be done on a version where the module didn't land yet, not in this one.
Technically, I still lean toward my suggestion, as it preserves existing behavior, whereas this PR introduces breaking changes for CJK users, which arguably goes against the intent of the stable policy.
That said, I’m fine with proceeding with this PR, considering the limited contribution from the CJK market. How about adding information to the ROADMAP file?
That said, I’m fine with proceeding with this PR, considering the limited contribution from the CJK market. How about adding information to the ROADMAP file?
I updated the roadmap. Please let me know if it's OK for you. Thanks.
On my way to merge this fine PR! Prepared branch 16.0-ocabot-merge-pr-1627-by-pedrobaeza-bump-patch, awaiting test results.
Congratulations, your PR was merged at 9ea048a20883631c6005a73640cac0f27ff595cf. Thanks a lot for contributing to OCA. ❤️