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

Cannot chain more than one call to a pg_search scope #1

Closed
pivotal-casebook opened this Issue Jan 19, 2011 · 19 comments

Comments

@pivotal-casebook

pivotal-casebook commented Jan 19, 2011

ruby-1.8.7-p174 :001 > BlogPost.search("foo").search("bar")
ActiveRecord::StatementInvalid: PGError: ERROR:  ORDER BY "pg_search_rank" is ambiguous
@avinasha

This comment has been minimized.

Show comment
Hide comment
@avinasha

avinasha Sep 21, 2011

Even something like BlogPost.search("foo").includes(:comments) raises the same error...

avinasha commented Sep 21, 2011

Even something like BlogPost.search("foo").includes(:comments) raises the same error...

@avinasha

This comment has been minimized.

Show comment
Hide comment
@avinasha

avinasha Sep 21, 2011

Ignore the above comment... my bad..

avinasha commented Sep 21, 2011

Ignore the above comment... my bad..

@skryl

This comment has been minimized.

Show comment
Hide comment
@skryl

skryl Jan 22, 2012

More specifically, chaining scopes that go through associations doesn't work. Anything that uses :associated_against doesn't generate the correct sql when chained. Chaining simple :against scopes seems to be working fine.

skryl commented Jan 22, 2012

More specifically, chaining scopes that go through associations doesn't work. Anything that uses :associated_against doesn't generate the correct sql when chained. Chaining simple :against scopes seems to be working fine.

@dzelle

This comment has been minimized.

Show comment
Hide comment
@dzelle

dzelle Jul 19, 2012

I also have difficulty when trying to chain a call to joins: Story.search("foo").joins(:posts).where(:posts = > {:user_id => 1}). Has any progress been made on this?

dzelle commented Jul 19, 2012

I also have difficulty when trying to chain a call to joins: Story.search("foo").joins(:posts).where(:posts = > {:user_id => 1}). Has any progress been made on this?

@davearonson

This comment has been minimized.

Show comment
Hide comment
@davearonson

davearonson Sep 8, 2012

I've found a workaround: use reorder. Not just plain order, which will tack yet another item onto the ORDER BY list, but REorder, which will CLEAR the ORDER BY list and put in a new item. I'm building a job board, and my query now look like:

Job.newer_than(age).
    with_description(description).
    with_title(title).
    within_distance(radius, lat, lng).
    reorder("posted_at DESC")

where newer_than and within_distance are normal scopes, but with_description and with_title are wrappers around pg_search_scopes (to take care of how those don't deal well with being passed blank or nil).

davearonson commented Sep 8, 2012

I've found a workaround: use reorder. Not just plain order, which will tack yet another item onto the ORDER BY list, but REorder, which will CLEAR the ORDER BY list and put in a new item. I'm building a job board, and my query now look like:

Job.newer_than(age).
    with_description(description).
    with_title(title).
    within_distance(radius, lat, lng).
    reorder("posted_at DESC")

where newer_than and within_distance are normal scopes, but with_description and with_title are wrappers around pg_search_scopes (to take care of how those don't deal well with being passed blank or nil).

@davearonson

This comment has been minimized.

Show comment
Hide comment

davearonson commented Sep 9, 2012

@nertzy

This comment has been minimized.

Show comment
Hide comment
@nertzy

nertzy Sep 9, 2012

Collaborator

@davearonson Thanks for the blog posts!

I just want to reiterate a good point from your second post to everyone else who reads this. Using reorder means that you won't actually be ranking by search relevance anymore. So you better be careful that you want that behavior. Essentially you will be getting all records that match the minimum threshold for your search. So the closest match could be anywhere in the list, even in last place, depending on what else you are using to sort results.

Collaborator

nertzy commented Sep 9, 2012

@davearonson Thanks for the blog posts!

I just want to reiterate a good point from your second post to everyone else who reads this. Using reorder means that you won't actually be ranking by search relevance anymore. So you better be careful that you want that behavior. Essentially you will be getting all records that match the minimum threshold for your search. So the closest match could be anywhere in the list, even in last place, depending on what else you are using to sort results.

@davearonson

This comment has been minimized.

Show comment
Hide comment
@davearonson

davearonson Sep 17, 2012

Update: I had an idea how this could be fixed, forked the code, and took a quick and dirty stab at it. Long story short, it could keep track of how many scopes it has applied, and use the number to create a unique tag on the end of each pg_search_rank. The top of scope_options.rb would then read:

require "active_support/core_ext/module/delegation"

module PgSearch
  class ScopeOptions
    attr_reader :config, :feature_options

    @@num_scopes_applied = 0

    def initialize(config)
      @config = config
      @model = config.model
      @feature_options = Hash[config.features]
    end

    def apply(scope)
      # TODO: write tests to make sure multi-ranking works right!
      # Alternate idea: make ranking at all, optional, perhaps per-scope.
      @@num_scopes_applied += 1
      rank_tag = @@num_scopes_applied == 1 ? "" : "_#{@@num_scopes_applied}"
      scope.select("#{quoted_table_name}.*, (#{rank}) AS pg_search_rank#{rank_tag}").where(conditions).order("pg_search_rank#{rank_tag} DESC, #{order_within_rank}").joins(joins)
    end

This seems to be working, when I sub it in on a project of mine. The generated SQL looks like:

SELECT "frobs".*, [gobbledygook] AS pg_search_rank, "frobs".*, [similar gobbledygook] AS pg_search_rank_2
FROM "frobs"
WHERE [more gobbledygook] AND [similar more gobbledygook]
ORDER BY pg_search_rank DESC, "frobs"."id" ASC, pg_search_rank_2 DESC, "frobs"."id" ASC

However, it breaks some tests (six, probably because of sometimes not resetting the number and thereby never having a plain search_rank), should have tests of its own to make sure multi-ranking works right, and I'm not sure that a class var is the right place to store this since I haven't looked into the code beyond finding where pg_search_rank comes from. That's why I'm not making a pull request quite yet. :-) Does it give those of you who know the rest of the code (and tests) well, a decent idea how to fix it? I could continue trying to grok the rest of the code and tests, but if this points the way well enough for you, I'll be happy to leave it in your hands.

davearonson commented Sep 17, 2012

Update: I had an idea how this could be fixed, forked the code, and took a quick and dirty stab at it. Long story short, it could keep track of how many scopes it has applied, and use the number to create a unique tag on the end of each pg_search_rank. The top of scope_options.rb would then read:

require "active_support/core_ext/module/delegation"

module PgSearch
  class ScopeOptions
    attr_reader :config, :feature_options

    @@num_scopes_applied = 0

    def initialize(config)
      @config = config
      @model = config.model
      @feature_options = Hash[config.features]
    end

    def apply(scope)
      # TODO: write tests to make sure multi-ranking works right!
      # Alternate idea: make ranking at all, optional, perhaps per-scope.
      @@num_scopes_applied += 1
      rank_tag = @@num_scopes_applied == 1 ? "" : "_#{@@num_scopes_applied}"
      scope.select("#{quoted_table_name}.*, (#{rank}) AS pg_search_rank#{rank_tag}").where(conditions).order("pg_search_rank#{rank_tag} DESC, #{order_within_rank}").joins(joins)
    end

This seems to be working, when I sub it in on a project of mine. The generated SQL looks like:

SELECT "frobs".*, [gobbledygook] AS pg_search_rank, "frobs".*, [similar gobbledygook] AS pg_search_rank_2
FROM "frobs"
WHERE [more gobbledygook] AND [similar more gobbledygook]
ORDER BY pg_search_rank DESC, "frobs"."id" ASC, pg_search_rank_2 DESC, "frobs"."id" ASC

However, it breaks some tests (six, probably because of sometimes not resetting the number and thereby never having a plain search_rank), should have tests of its own to make sure multi-ranking works right, and I'm not sure that a class var is the right place to store this since I haven't looked into the code beyond finding where pg_search_rank comes from. That's why I'm not making a pull request quite yet. :-) Does it give those of you who know the rest of the code (and tests) well, a decent idea how to fix it? I could continue trying to grok the rest of the code and tests, but if this points the way well enough for you, I'll be happy to leave it in your hands.

@davearonson

This comment has been minimized.

Show comment
Hide comment
@davearonson

davearonson Sep 17, 2012

I've been playing with a number of alternatives. The key to getting this thing working and not breaking existing tests, seems to be to reset that number upon creation of a new query. But, I'm not sufficiently familiar with what happens behind the scenes, to know where to hook that in.

I tried detecting whether the scope passed in to apply would respond to select_values, to see if it was just the raw class or an already-built query, but apparently the latter is never the case. (Or maybe I was detecting it wrong.) Tried Googling it, but couldn't find anything. Could possibly derive or monkeypatch, but those just seem unclean.

Suggestions?

davearonson commented Sep 17, 2012

I've been playing with a number of alternatives. The key to getting this thing working and not breaking existing tests, seems to be to reset that number upon creation of a new query. But, I'm not sufficiently familiar with what happens behind the scenes, to know where to hook that in.

I tried detecting whether the scope passed in to apply would respond to select_values, to see if it was just the raw class or an already-built query, but apparently the latter is never the case. (Or maybe I was detecting it wrong.) Tried Googling it, but couldn't find anything. Could possibly derive or monkeypatch, but those just seem unclean.

Suggestions?

@shanemcd

This comment has been minimized.

Show comment
Hide comment
@shanemcd

shanemcd Dec 14, 2012

Hey Dave! What did you end up going with?

shanemcd commented Dec 14, 2012

Hey Dave! What did you end up going with?

@davearonson

This comment has been minimized.

Show comment
Hide comment
@davearonson

davearonson Dec 14, 2012

My project is currently using pg_search, but I haven't put time recently into fixing pg_search's scope unchainability. Been too busy with paying work. :-) Thought someone else's recent fix was gonna do it, but no....

davearonson commented Dec 14, 2012

My project is currently using pg_search, but I haven't put time recently into fixing pg_search's scope unchainability. Been too busy with paying work. :-) Thought someone else's recent fix was gonna do it, but no....

@joemsak

This comment has been minimized.

Show comment
Hide comment
@joemsak

joemsak Apr 16, 2013

@davearonson You are my hero (found your slides on slideshare)

joemsak commented Apr 16, 2013

@davearonson You are my hero (found your slides on slideshare)

@monfresh

This comment has been minimized.

Show comment
Hide comment
@monfresh

monfresh May 5, 2014

Not sure why this issue was closed as I don't see an official fix for it. I see that @nertzy closed it by referencing issue #68, where it was assumed, but never confirmed, that merging scopes was a workaround. That "workaround" did not work for me with pg_search 0.7.3 and Rails 4.0.4, whether I chain scopes normally or with a merge. Here are my pg scopes:

pg_search_scope :keyword_search, against: :tsv_body,
    using: {
      tsearch: {
        dictionary: "english",
        any_word: false,
        prefix: true,
        tsvector_column: 'tsv_body'
      }
    }

pg_search_scope :search_by_language, :against => :languages

If I do Location.keyword_search('food').merge(Location.search_by_language('french')), I get:

PG::AmbiguousColumn: ERROR:  ORDER BY "pg_search_rank" is ambiguous

If I do `Location.keyword_search('food').search_by_language('french')), I also get the same error:

PG::AmbiguousColumn: ERROR:  ORDER BY "pg_search_rank" is ambiguous

Has anyone gotten this to work?

monfresh commented May 5, 2014

Not sure why this issue was closed as I don't see an official fix for it. I see that @nertzy closed it by referencing issue #68, where it was assumed, but never confirmed, that merging scopes was a workaround. That "workaround" did not work for me with pg_search 0.7.3 and Rails 4.0.4, whether I chain scopes normally or with a merge. Here are my pg scopes:

pg_search_scope :keyword_search, against: :tsv_body,
    using: {
      tsearch: {
        dictionary: "english",
        any_word: false,
        prefix: true,
        tsvector_column: 'tsv_body'
      }
    }

pg_search_scope :search_by_language, :against => :languages

If I do Location.keyword_search('food').merge(Location.search_by_language('french')), I get:

PG::AmbiguousColumn: ERROR:  ORDER BY "pg_search_rank" is ambiguous

If I do `Location.keyword_search('food').search_by_language('french')), I also get the same error:

PG::AmbiguousColumn: ERROR:  ORDER BY "pg_search_rank" is ambiguous

Has anyone gotten this to work?

@amnesia7

This comment has been minimized.

Show comment
Hide comment
@amnesia7

amnesia7 Jun 26, 2014

Has anyone managed to find a solution to this issue yet?

I don't think I've got the know-how to contribute code but the suggestion by @davearonson to increment a number for each chained search call to make each pg_search_rank column unique sounds about right - keeping the initial one as pg_search_rank and subsequent ones as pg_search_rank_X so as not to break anyone's code that has manually declared pg_search_rank when ordering.

I saw the slides but they only advised removing the pg_search_rank sorting column completely which doesn't help me.

amnesia7 commented Jun 26, 2014

Has anyone managed to find a solution to this issue yet?

I don't think I've got the know-how to contribute code but the suggestion by @davearonson to increment a number for each chained search call to make each pg_search_rank column unique sounds about right - keeping the initial one as pg_search_rank and subsequent ones as pg_search_rank_X so as not to break anyone's code that has manually declared pg_search_rank when ordering.

I saw the slides but they only advised removing the pg_search_rank sorting column completely which doesn't help me.

@bcackerman

This comment has been minimized.

Show comment
Hide comment
@bcackerman

bcackerman Oct 29, 2014

@davearonson what's inside your within_distance method?

bcackerman commented Oct 29, 2014

@davearonson what's inside your within_distance method?

@davearonson

This comment has been minimized.

Show comment
Hide comment
@davearonson

davearonson Oct 29, 2014

@bcackerman I trashed that project long ago so I can't check, but IIRC it was something like "convert the radius into a number of degrees latitude and a number of degrees longitude (which may vary according to what the latitude is), and use ordinary math pick out the jobs in the resulting square centered on the desired place". Yes, that means it included some that weren't really within the radius, but it was a lot faster than doing all the Pythagorean math, let alone trig.

davearonson commented Oct 29, 2014

@bcackerman I trashed that project long ago so I can't check, but IIRC it was something like "convert the radius into a number of degrees latitude and a number of degrees longitude (which may vary according to what the latitude is), and use ordinary math pick out the jobs in the resulting square centered on the desired place". Yes, that means it included some that weren't really within the radius, but it was a lot faster than doing all the Pythagorean math, let alone trig.

@nertzy

This comment has been minimized.

Show comment
Hide comment
@nertzy

nertzy Apr 28, 2015

Collaborator

#240 introduces a failing spec if anyone wants to try their hands at a fix.

I merged the spec into the issue-1 branch.

Collaborator

nertzy commented Apr 28, 2015

#240 introduces a failing spec if anyone wants to try their hands at a fix.

I merged the spec into the issue-1 branch.

@doits

This comment has been minimized.

Show comment
Hide comment
@doits

doits Apr 28, 2015

Contributor

Should be possible to overcome this by making pg_search.rank unique. The only problem I see is that it is hardcoded in WithPgSearchRank module, so this would have to be changed first to not use a module like this to extend the scope but something coming from the ScopeOptions instance ...

Contributor

doits commented Apr 28, 2015

Should be possible to overcome this by making pg_search.rank unique. The only problem I see is that it is hardcoded in WithPgSearchRank module, so this would have to be changed first to not use a module like this to extend the scope but something coming from the ScopeOptions instance ...

@budu

This comment has been minimized.

Show comment
Hide comment
@budu

budu May 21, 2015

Contributor

I've written a fix inspired by @davearonson solution for this issue as I'm currently working on a project with rather complex uses of pg_search in combination with ransack. All tests are passing and I've written two more, based on @nertzy work. The gem behavior should be only minimally affected, enjoy!

More details in the commit message.

Contributor

budu commented May 21, 2015

I've written a fix inspired by @davearonson solution for this issue as I'm currently working on a project with rather complex uses of pg_search in combination with ransack. All tests are passing and I've written two more, based on @nertzy work. The gem behavior should be only minimally affected, enjoy!

More details in the commit message.

NikoRoberts pushed a commit to airtasker/pg_search that referenced this issue Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment