search

pat / thinking-sphinx

Sphinx/Manticore plugin for ActiveRecord/Rails
http://freelancing-gods.com/thinking-sphinx
MIT License
1.63k stars 468 forks source link

Array.concat() doesn't work with Search in version 2 #237

Closed tjoneseng closed 13 years ago

tjoneseng commented 13 years ago

Here's a code snippet:

results = []
sphinx_results = MyModel.search(...)
sphinx_results.empty?    # false: we got some results
results.concat(sphinx_results)
results.empty?    # true: fail!

I think "populate" needs to be called on the result set before it is concat'd into the results array above. If Search is going to extend Array then its semantics should be consistent.

pat commented 13 years ago

It'd be nice if this would work - but given Array#concat is part of Ruby itself, it's possible that the underlying implementation is written in C and not using any of the standard Array element accessors.

Even so, all the standard Array accessors work with ThinkingSphinx::Search - and populate is called before all of them. Even in the example you provided, populate is called when you call #empty? - so there's results loaded before you call concat.

As a workaround, you should be able to use the following instead: results.concat(sphinx_results.to_a)

I'm curious though - why do you want to concat search results?

On 13/06/2011, at 1:56 AM, barbarycodes wrote:

Here's a code snippet:

results = [] sphinx_results = MyModel.search(...) sphinx_results.empty? # false: we got some results results.concat(sphinx_results) results.empty? # true: fail!

I think "populate" needs to be called on the result set before it is concat'd into the results array above. I'm not sure how that would happen automatically, aside from monkey-patching Array. I realize the value of lazy-loading, but if Search is going to extend Array then it's semantics should be consistent.

Reply to this email directly or view it on GitHub: https://github.com/freelancing-god/thinking-sphinx/issues/237

tjoneseng commented 13 years ago

You're right -- the pseudocode was fudged a bit -- I'm not actually calling empty? before concat.

I want to use concat because some of my data comes from Sphinx and some from other sources.

On Jun 13, 2011, at 5:42 AM, freelancing-god wrote:

It'd be nice if this would work - but given Array#concat is part of Ruby itself, it's possible that the underlying implementation is written in C and not using any of the standard Array element accessors.

Even so, all the standard Array accessors work with ThinkingSphinx::Search - and populate is called before all of them. Even in the example you provided, populate is called when you call #empty? - so there's results loaded before you call concat.

As a workaround, you should be able to use the following instead: results.concat(sphinx_results.to_a)

I'm curious though - why do you want to concat search results?

On 13/06/2011, at 1:56 AM, barbarycodes wrote:

Here's a code snippet:

results = [] sphinx_results = MyModel.search(...) sphinx_results.empty? # false: we got some results results.concat(sphinx_results) results.empty? # true: fail!

I think "populate" needs to be called on the result set before it is concat'd into the results array above. I'm not sure how that would happen automatically, aside from monkey-patching Array. I realize the value of lazy-loading, but if Search is going to extend Array then it's semantics should be consistent.

Reply to this email directly or view it on GitHub: https://github.com/freelancing-god/thinking-sphinx/issues/237

Reply to this email directly or view it on GitHub: https://github.com/freelancing-god/thinking-sphinx/issues/237#issuecomment-1358582

pat commented 13 years ago

Right, well even if you do force the results to be populated, concat doesn't work. I'd recommend the workaround of calling to_a on sphinx_results.

On 14/06/2011, at 2:09 AM, barbarycodes wrote:

You're right -- the pseudocode was fudged a bit -- I'm not actually calling empty? before concat.

I want to use concat because some of my data comes from Sphinx and some from other sources.

On Jun 13, 2011, at 5:42 AM, freelancing-god wrote:

It'd be nice if this would work - but given Array#concat is part of Ruby itself, it's possible that the underlying implementation is written in C and not using any of the standard Array element accessors.

Even so, all the standard Array accessors work with ThinkingSphinx::Search - and populate is called before all of them. Even in the example you provided, populate is called when you call #empty? - so there's results loaded before you call concat.

As a workaround, you should be able to use the following instead: results.concat(sphinx_results.to_a)

I'm curious though - why do you want to concat search results?

On 13/06/2011, at 1:56 AM, barbarycodes wrote:

Here's a code snippet:

results = [] sphinx_results = MyModel.search(...) sphinx_results.empty? # false: we got some results results.concat(sphinx_results) results.empty? # true: fail!

I think "populate" needs to be called on the result set before it is concat'd into the results array above. I'm not sure how that would happen automatically, aside from monkey-patching Array. I realize the value of lazy-loading, but if Search is going to extend Array then it's semantics should be consistent.

Reply to this email directly or view it on GitHub: https://github.com/freelancing-god/thinking-sphinx/issues/237

Reply to this email directly or view it on GitHub: https://github.com/freelancing-god/thinking-sphinx/issues/237#issuecomment-1358582

Reply to this email directly or view it on GitHub: https://github.com/freelancing-god/thinking-sphinx/issues/237#issuecomment-1359837