ActiveRecord Single Table Inheritance search bug #218

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

sandrew commented Jan 30, 2012

When I need to make a search on STI ancestor (to get mixed results from all descendants) on one index (like in tests), I was usually getting error, because it makes 'find' from wrong class. Well, you can see it in tests.

Also it repaires one more bug: when you have two STI classes with separate indexes (like in default) and try to make a search like Tire.search('first_table_index,second_table_index', :load => true) { ... }, it also raises RecordNotFound.

Owner

karmi commented Jan 31, 2012

Hi Andrew, thanks for taking your time to have a look into this. Sadly, this issue has been debated over and over -- please search the issues --, and there are now several patches trying to solve several problems related to this.

The thing is, I feel the codebase right now is too fragile to out it under more load. I like the way you're going in your patch, though, and will definitely look at it when doing some refactorings in February (hopefully).

Owner

karmi commented Mar 1, 2012

@gearhead90 I like how you approach the problem, Andrew! I generally try to avoid ActiveSupport like a plague, but the group_by here works like a charm. I think this is a viable solution. I'll test it as the solution to "multi model searches" and will get back. Please ping me here or on IRC if I'll fail to do that in couple of weeks.

Contributor

sandrew commented Mar 18, 2012

Hello again. I'd like to remind you to make a decision on this request.

Owner

karmi commented Mar 18, 2012

@sandrew Hi, I like how you approach the problem and will try to incorporate it to the codebase the moment I'll start working on it. If nothing blocks the implementation, I hope we can have it in this week.

allochi commented Mar 21, 2012

It would be great if we can have it this week! I'm stuck at this point :D

I have a class I call Resource, and it has 3 children Document, Image and WebPage (and more later), I'm doing all the mapping in Resource class, which is an ActiveRecord class, but it doesn't work, when I force index using rake it just skip the mapping and index all properties.

I wish that this get fixed really soon, so that I don't have to rewrite all my classes again to make Tire work. Please Help :)

Oh, many many thanks for Tire!

I just deleted my comment on this being fixed in the 0.4.0pre, sorry for that, it worked when I set the type to be null in the database, which means no more inheritance.

karmi closed this in 9ded21a Mar 22, 2012

@karmi karmi added a commit that referenced this pull request Mar 22, 2012

@karmi @karmi karmi + karmi [#218] Cleaned up the codebase for loading instances of multiple clas…
…ses / single table inheritance

Previously, eagerly loading instances of multiple classes with the `load: true` option was not possible.

Now, you can use the `Tire.search` DSL for searching and loading multiple models from one or several indices:

    ActiveRecordModelOne.create :title => 'Title One', timestamp: Time.now.to_i

    s = Tire.search ['active_record_model_one', 'active_record_model_two'], :load => true do
                  query { string 'title' }
                  sort  { by :timestamp }
                end

    puts s.results[0].inspect
    # => <ActiveRecordModelOne id: 1, title: "Title One", timestamp: "1332437294">

Fixed indentation/formatting/whitespace throughout the suite :/

Fixes #80, fixes #178, fixes #218, fixes #277, closes #131.
d5c08fb

@karmi karmi added a commit that referenced this pull request Mar 27, 2012

@sarmiena @karmi sarmiena + karmi [#218] Fixed the incorrect serialization of `document_type` in Index#…
…bulk_store

Previously, when storing individual namespaced records, it correctly stored them as "my_namespace/my_model",
but bulk_store was saving _type as "my_namespace%2Fmy_model".

This resulted in two different document types where there should only be one, incorrect mapping, incorrect searches.

Closes #290, closes #291, fixes #218.
4a92865

@Evan-M Evan-M added a commit to Evan-M/tire that referenced this pull request Apr 21, 2012

@Evan-M Evan-M Merge remote-tracking branch 'upstream/master'
* upstream/master: (39 commits)
  Release 0.4.0
  [GEMS] Relaxed gem dependencies for "rest-client", "multi_json" and "bundler"
  Release 0.4.0.rc
  [#218] Cleaned up the test suite for document_type in Index#bulk_store
  [#218] Fixed the incorrect serialization of `document_type` in Index#bulk_store
  [FIX] Fixed displaying of Rake task usage
  [#289] Update README and documentation, use the `prefix` query as the example of "unsupported" query
  [#289] Added an example of fuzzy query in the Text query integration test
  [TEST] Improved code in the Text Query integration test
  [TEST] Improved code legibility in the Explanation integration test
  [TEST] Improved code legibility in the DSL integration test
  [#289] Cleaned up the code for "term" and "fuzzy" queries and restructured the search query test suite
  [#289] Improvements to the "term" query type
  [#289] Added the "fuzzy" query type
  [GEMS] Updated and cleaned up gem dependencies
  Restructured the test suite for Ruby 1.8 unit test compatibility
  Added Ruby 1.8 compatibility for `Utils.escape/unescape`
  [TEST] Removed the "supermodel" gem and used the "redis-persistence" gem
  [ACTIVEMODEL] Fixed and cleaned up URL-escaping of document type
  [UTILS] Added the `Tire::Utils` module
  ...
7eb34b0

karmi referenced this pull request Jul 25, 2012

Closed

STI bug with geo_point field #394

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