FEATURE: Adds option to search only in titles #5538

jorgemanrubia · 2018-01-27T12:59:32Z

It adds a new search option in:title that will make it search only in titles.

It also adds a new checkbox at the top of the advanced search UI:

See:

Overview of changes

It changes the way posts and topics are indexed to use PostgreSQL full-text search weights. Then, it uses the weight associated with title to filter by title when searching posts.

Before, it was concatenating title + body + category and storing the indexed result. Now, it will store the concatenation of weighted indexed parts.

This is my first experience with PostgreSQL full-text search. I think it is the proper way to do it after checking the docs. The weights system is a little bit bizarre with 4 values (A, B, C, D) but seems to work pretty well.

As a side effect, results will take weight into consideration when sorting. So title > body > category. From 12.3.3. Ranking Search Results:

If no weights are provided, then these defaults are used:
{0.1, 0.2, 0.4, 1.0}

So for posts, weights will be 1.0 for title title, 0.4 for body and 0.2 for category. I think this is ok, but it's definitely something good to check before merging the PR. I tested this locally with a database with 19k posts from a forum I used to run, and the sorting made sense to me.

I will add some comments in the code to explain the changes next.

Reindexing required

in:title will only work after re-indexing all the posts. Not sure how to do it automatically when shipping this:

Should it trigger a reindexing job as part of the deploy? Is there support in place for such tasks? I guess we could launch a job from a migration if not
Should it just increase the INDEX_VERSION and rely on the indexing job that runs every day?

In development I have used rake search:reindex for testing this.

discoursebot · 2018-01-27T12:59:36Z

You've signed the CLA, jorgemanrubia. Thank you! This pull request is ready for review.

jorgemanrubia · 2018-01-27T13:12:47Z

app/services/search_indexer.rb

+  # first one will have a priority A; he second one a priority B, etc.
+  # When only 1 entry is provided, no weights will be used.
+  def self.update_index(table, id, *raw_entries)
+    raw_data = Search.prepare_data(raw_entries.join(' '), :index)


This method now supports providing multiple entries to index and, when more than 1 entry is provided, it will assign them a weight.

I didn't add support for providing the specific weights and, instead, used the convention of deducing it from the order. So the first entry gets an A, the second a B, and so. I think this works well for what we need and keeps the code simpler.

jorgemanrubia · 2018-01-27T13:14:30Z

app/services/search_indexer.rb

+      end.join(" || ")
+    end
+  end
+
  def self.update_topics_index(topic_id, title, cooked)


While weights were only needed for posts, I added them for topics too for consistency

discoursebot · 2018-01-27T13:21:28Z

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/search-only-within-topic-titles/27600/10

jorgemanrubia · 2018-01-27T13:22:47Z

app/services/search_indexer.rb

    # don't allow concurrency to mess up saving a post
  end

+  # for user login and name use "simple" lowercase stemmer


I extracted a few methods from SearchIndexer#update_index

jorgemanrubia · 2018-01-27T13:23:39Z

app/assets/javascripts/discourse/components/search-advanced-options.js.es6

-  updateSearchTermForSpecialInLikes() {
-    const match = this.filterBlocks(REGEXP_SPECIAL_IN_LIKES_MATCH);
-    const inFilter = this.get('searchedTerms.special.in.likes');
+  updateSearchTermForSpecialIn(key, regexp){


Extracted method to remove some duplicated code for the "searching in" logic

jorgemanrubia · 2018-01-27T13:25:42Z

app/services/search_indexer.rb

+    "indexable_fragment_#{index}".to_sym
+  end
+
+  def self.build_ts_vector(stemmer, indexable_entries)


This is the place where we add weights to entries when more than 1 entry is provided. When only 1 entry is provided (users, categories) it works like it used to.

jorgemanrubia · 2018-01-27T15:36:14Z

When searching exact terms with "..." it won't work with titles. This is already happening in current version: titles are ignored when performing exact searches.

The reason is that these queries check posts.raw, which does not include the title. I can tweak that and use post_search_data.raw_data which is the concatenation of title+cooked+category name but wasn't really sure if we want to tweak that.

SamSaffron · 2018-01-28T06:12:42Z

That 1.freeze I may have added there sure is odd, I would just bump version to 2 and rely on it the indexing job.

Can you review the existing code that attempts to give a bump to title. Perhaps there is a bunch of code that can be removed now? eg: https://github.com/discourse/discourse/blob/master/lib/search.rb#L746

I think that checkbox in the UI has a bit too much prominence I would bump it to the bottom of the advanced area.

Overall the PR looks great, nice feature.

jorgemanrubia · 2018-01-28T09:32:14Z

@SamSaffron I added some commits addressing your comments, thanks!

Regarding the ordering, this commit will use the new weights to bump titles. With the default weights of {0.1, 0.2, 0.4, 1.0} it wasn't bumping titles as in the current implementation, where title matches always appear first. I experimented with a few values and making the relation 1 to 0.1 keeps the same behaviour.

A problem with this change is that, until the database is reindexed, title bumping in results won't work. Not sure if that's ok. If it is not, we can rollback that change and deploy it a few days after getting this shipped to be safe.

jorgemanrubia · 2018-01-28T09:42:39Z

I was having a look at Job::ReindexSearch and I am seeing that it has a limit of 10000 for posts and that it only fetches a list of ids. I wonder if that limit is too short for very active communities (such as Meta). I understand it's for performance reasons, but what about getting rid of the limit, adding a select for only loading ids into the records, and then using find_each to process them in batches and prevent memory issues?

Something like:

posts = Post.joins(:topic)
            .select(:id)
            .where(...)

posts.find_each {|post| indexing_stuff}

SamSaffron · 2018-02-20T03:47:17Z

OK, I took some time today to re-do this work based on your awesome changes. per: 86d12bd

I opted to hack this in myself cause I was very concerned about maintenance and needed to know exactly how everything works together.

I opted out of UI changes for now for a couple more betas so the index has some time to rebuild.

Overall I was very happy with this change and I think it provides discourse with a significantly better search.

ranking wise I went with title category tags body this seems to be doing the trick.

coding-horror · 2018-02-20T05:00:47Z

yes this was an excellent nudge to improve search on titles which we did get a lot of complaints about .. glad we were actually doing something wrong, that makes it easier to fix.. thank you @jorgemanrubia

jorgemanrubia added 7 commits January 27, 2018 07:15

Add support for indexing multiple entries with weights

b71c004

Use the new system of weights with posts too

5543ccf

Basics of search only in title working

2580518

Add spec for in:title

6b6077c

Add checkbox for searching in titles only

bf6064c

Refactor: reuse method and remove duplicated code

a7603be

Add acceptance tests

770950a

Add doc

848cfdc

jorgemanrubia commented Jan 27, 2018

View reviewed changes

jorgemanrubia changed the title ~~[FEATURE] Adds option to search only in titles~~ FEATURE: Adds option to search only in titles Jan 27, 2018

jorgemanrubia added 3 commits January 27, 2018 15:26

Rubocop fixes

96a2b21

Rubocop fixes

5093fcd

Change error message

ff608a4

jorgemanrubia added 6 commits January 28, 2018 09:41

Update doc

0d1abe0

Rename for consistency

e880e62

Bump index version after changing the indexing approach to use weights

7d7ba30

Place "in:title" checkbox at the bottom to make it less prominent

a6a0bf8

Bump titles using posgtresql full text search weights

74fc6d8

Tweak comment

b8c1983

SamSaffron closed this Feb 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEATURE: Adds option to search only in titles #5538

FEATURE: Adds option to search only in titles #5538

jorgemanrubia commented Jan 27, 2018 •

edited

discoursebot commented Jan 27, 2018

jorgemanrubia Jan 27, 2018 •

edited

jorgemanrubia Jan 27, 2018

discoursebot commented Jan 27, 2018

jorgemanrubia Jan 27, 2018

jorgemanrubia Jan 27, 2018

jorgemanrubia Jan 27, 2018

jorgemanrubia commented Jan 27, 2018

SamSaffron commented Jan 28, 2018

jorgemanrubia commented Jan 28, 2018

jorgemanrubia commented Jan 28, 2018 •

edited

SamSaffron commented Feb 20, 2018

coding-horror commented Feb 20, 2018

FEATURE: Adds option to search only in titles #5538

FEATURE: Adds option to search only in titles #5538

Conversation

jorgemanrubia commented Jan 27, 2018 • edited

Overview of changes

Reindexing required

discoursebot commented Jan 27, 2018

jorgemanrubia Jan 27, 2018 • edited

Choose a reason for hiding this comment

jorgemanrubia Jan 27, 2018

Choose a reason for hiding this comment

discoursebot commented Jan 27, 2018

jorgemanrubia Jan 27, 2018

Choose a reason for hiding this comment

jorgemanrubia Jan 27, 2018

Choose a reason for hiding this comment

jorgemanrubia Jan 27, 2018

Choose a reason for hiding this comment

jorgemanrubia commented Jan 27, 2018

SamSaffron commented Jan 28, 2018

jorgemanrubia commented Jan 28, 2018

jorgemanrubia commented Jan 28, 2018 • edited

SamSaffron commented Feb 20, 2018

coding-horror commented Feb 20, 2018

jorgemanrubia commented Jan 27, 2018 •

edited

jorgemanrubia Jan 27, 2018 •

edited

jorgemanrubia commented Jan 28, 2018 •

edited