clear-code / redmine_full_text_search

Full text search for Redmine
MIT License
61 stars 24 forks source link

Wrong reorder for Wiki page search #7

Closed kou closed 7 years ago

kou commented 7 years ago

lib/full_text_search/mroonga.rbfetch_ranks_and_ids

joins("INNER JOIN fts_wiki_contents ON (wiki_contents.id = fts_wiki_contents.wiki_content_id)").
reorder("score1 DESC, score2 DESC").distinct.limit(limit).map do |record|
if order_target == "score"
  [record.score1 * 100 + record.score2, record.id]
else
  [record.order_target.to_i, record.id]
end

と書いているけど、order_target"score"じゃないときはreorder("order_target, id")にしないといけないと思う。

そうしないと、limit10で実際にヒットした件数が100件のときに違う結果が返ると思う。

kou commented 7 years ago

あと、現状のreorderidが抜けていると思う。

okkez commented 7 years ago

ここで reorder しても、最後に Redmine 側で sort! するようになっているのでした。 なのでここは間違えていないはずです。

See: https://github.com/redmine/redmine/blob/master/lib/redmine/search.rb#L130

kou commented 7 years ago

Redmine側でソートするならそもそもreorderしなくていいんじゃないかなぁと思いました。

kou commented 7 years ago

limit10で実際にヒットした件数が100件のときに違う結果が返ると思う。

このケースも問題ないということですか?

私は問題があると思うんですけどねぇ。

a = 100.times.collect {rand(100)}
# 100件レコードを生成
# => [18, 84, 77, 20, 62, 53, 12, 0, 88, 60, 72, 49, 40, 94, 34, 10, 22, 43, 14, 55, 90, 65, 61, 82, 41, 47, 16, 11, 8, 59, 60, 69, 80, 27, 98, 51, 31, 46, 23, 15, 0, 15, 14, 57, 16, 99, 83, 69, 8, 7, 38, 17, 71, 18, 17, 91, 61, 44, 60, 94, 60, 34, 91, 49, 29, 65, 38, 85, 81, 54, 87, 99, 1, 96, 56, 8, 72, 1, 37, 76, 91, 81, 78, 16, 32, 0, 31, 19, 86, 57, 58, 38, 14, 89, 72, 38, 79, 23, 35, 78]
a.sort[0..10]
# 100件のレコード全体でソートしてから先頭10件を取得
# => [0, 0, 0, 1, 1, 7, 8, 8, 8, 10, 11]
a[0..10].sort
# 10件のレコードを取得してからソート
# => [0, 12, 18, 20, 53, 60, 62, 72, 77, 84, 88]
# ↑の2つで結果が異なる
okkez commented 7 years ago

Redmine側でソートするならそもそもreorderしなくていいんじゃないかなぁと思いました。

SQL で limit 付いてるので reorder は必要です。 無いと返ってくる結果が一定になりません。

okkez commented 7 years ago

そもそも LIMIT 句付きのクエリが一切発行されてなかった。 なので、ヒットした全件を取得してから id の配列からソートしてる。。。 つまり reorder も必要ないと言えば必要ない。たぶん元にした Redmine の実装で付いてたから付けてただけっぽい。

okkez commented 7 years ago

won't fix.