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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow simultaneously searching on both tsvector & non-tsvector columns #152

Closed
wants to merge 1 commit into from
Closed

Conversation

ajb
Copy link
Contributor

@ajb ajb commented Dec 26, 2013

Opening this again, tests included this time. (#126 was the old PR)

@nertzy would appreciate you taking a look when you get a chance :). Happy holidays! 馃巺

column_name = connection.quote_column_name(options[:tsvector_column])
"#{quoted_table_name}.#{column_name}"
else
columns.map do |search_column|
return_str = columns.map do |search_column|
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's gotta be better variable name than return_str...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A better approach would be to create an array of expressions and then only do the .join(" || ") part at the very end.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 011c78c on adamjacobbecker:from-upstream into c10704a on Casecommons:master.

@nertzy
Copy link
Collaborator

nertzy commented Jan 5, 2014

I'm a bit wary of merging this in. I think I need to better understand your use case.

For one thing, you're changing the current behavior of how :against and :tsvector_column interact, which would be a breaking change for existing users. For example, this valid search scope would now behave differently:

pg_search_scope :search_content,
  :against => "title",
  :using => {
    :tsearch => {
      :tsvector_column => "title_tsvector"
    },
    :trigram => {},
    :dmetaphone => {}
  }

In your implementation, I'm not sure how it would be possible to tell :trigram and :dmetaphone to search against title while having :tsearch only search against tsvector_title.

Also, I want to understand why you need to mix the two column types together. I feel like the most common case for using :tsvector_column is to write a database trigger that keeps the tsvector column up-to-date with all searchable content on every insert, like it's described in http://www.postgresql.org/docs/current/static/textsearch-features.html#TEXTSEARCH-UPDATE-TRIGGERS

Let me know if that solution doesn't work for your particular use case, so that we can figure out how best to make pg_search work for you. For now, I'm closing this pull request. Feel free to reopen it if there's something I've misunderstood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants