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

feat(backend): pgroongaに対応(configの構成変更あり) #14978

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

samunohito
Copy link
Member

@samunohito samunohito commented Nov 17, 2024

※2024.11.0の差分もそこそこたまってきているのでこれより後のリリースにしたい

What

  • pgroonga導入済みの場合、pgroongaのオペレータ(&@)でノート検索が出来るようになります
  • configの書式に少々変更があります。全文検索用のプロバイダを明示する必要があります。
    meilisearchを利用している場合、configのfulltextSearch.providermeilisearchを明示する必要があります(明示しない場合、SQLのLIKE検索が使用されます)
    meilisearchを利用していないのであれば特に変更はありません。

Why

fix #14730

Additional info (optional)

  • note.textにpgroongaのindexを作成し、実行計画が改善されることを確認(自鯖/214327レコード程度)
    before
    image
    after
    image

  • 自分のサーバにpgroongaとこのprの内容を先行導入し、実際に動作確認

  • 下記理由からpgroongaのindexを付与するマイグレーションsqlを作成していません

    • pgroongaはPostgreSQLの拡張機能であり、デフォルトでは存在しないものである
      (インストールしてPostgreSQLに認識させないとインデックスを作れない)
    • マイグレーションsqlの中でpgroongaの導入有無を判定するのはちょっと現実的ではない
  • 導入する場合、下記のようなSQLを手動実行してindexを作成いただく必要があります。

create index idx_note_text_with_pgroonga on note using pgroonga (text);

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Nov 17, 2024
Copy link
Contributor

github-actions bot commented Nov 17, 2024

このPRによるapi.jsonの差分
差分はありません。
Get diff files from Workflow Page

Copy link

codecov bot commented Nov 17, 2024

Codecov Report

Attention: Patch coverage is 28.19149% with 135 lines in your changes missing coverage. Please review.

Project coverage is 39.97%. Comparing base (a3d236c) to head (d6df98b).
Report is 449 commits behind head on develop.

Files with missing lines Patch % Lines
packages/backend/src/core/SearchService.ts 23.83% 131 Missing ⚠️
packages/backend/src/GlobalModule.ts 42.85% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14978      +/-   ##
===========================================
- Coverage    41.74%   39.97%   -1.78%     
===========================================
  Files         1549     1563      +14     
  Lines       196555   197822    +1267     
  Branches      2767     3635     +868     
===========================================
- Hits         82055    79072    -2983     
- Misses      113939   118145    +4206     
- Partials       561      605      +44     

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


🚨 Try these New Features:

@fruitriin
Copy link
Contributor

芸細な話でいうと、

query.andWhere('note.text ILIKE :q', { q: `%${sqlLikeEscape(q)}%` });

ここを note.cw + note.text みたいにして cwも検索対象に含める(インデックスは cwとtextを結合したものをあらかじめ作っておく)と個人的により神です

@samunohito
Copy link
Member Author

samunohito commented Nov 17, 2024

ここを note.cw + note.text みたいにして cwも検索対象に含める(インデックスは cwとtextを結合したものをあらかじめ作っておく)と個人的により神です

related: #14373

@samunohito
Copy link
Member Author

samunohito commented Nov 17, 2024

explain analyse select * from note where "userId" = 'id' and text &@ 'text';

explain analyse select * from note where "userId" = 'id' and (array[text, cw] &@ 'text');

とすることでcwを含めた検索も可能となりますが、挙動が明確に変わるので別対応とします。

また、上記クエリに対応させるとindexの作成方法も変わってきます…

CREATE INDEX idx_note_text_and_cw_combined_with_pgroonga ON note USING pgroonga ((array[text, cw]));

@samunohito samunohito marked this pull request as ready for review November 17, 2024 03:33
@fruitriin
Copy link
Contributor

fruitriin commented Nov 17, 2024

実際導入してみてnitsな指摘ですが、コンパネから現在の検索プロバイダをどのモードとして認識しているか確認できると嬉しいかもと思いました
(configしたつもりで認識されてないわーとか起きそうだなと)
まあ熟練のサーバー管理人ならそんな失敗しないかもしれませんけど!
(私はconfigだけいじってコードの適用を忘れていましたの札)

@samunohito
Copy link
Member Author

いいアイデアだと思います。
…が、実現するにはconfigの値をフロントエンドまで伝搬させる必要があり、現実的ではなさそうです。

代わりに、プロバイダの名称をサーバ起動時にログ出力するようにしたのですが、多少は改善されるでしょうか…
image

@fruitriin
Copy link
Contributor

十分だと思います!ありがとうございます

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

検索機能をpgroongaに対応させる
2 participants