Skip to content
This repository has been archived by the owner on Jun 30, 2018. It is now read-only.

ActiveRecord Single Table Inheritance search bug #218

Closed
wants to merge 2 commits into from
Closed

ActiveRecord Single Table Inheritance search bug #218

wants to merge 2 commits into from

Conversation

sandrew
Copy link
Contributor

@sandrew 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.

@karmi
Copy link
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).

@karmi
Copy link
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.

@sandrew
Copy link
Contributor Author

sandrew commented Mar 18, 2012

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

@karmi
Copy link
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
Copy link

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 karmi closed this in 9ded21a Mar 22, 2012
karmi added a commit that referenced this pull request Mar 22, 2012
…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.
karmi pushed a commit that referenced this pull request Mar 27, 2012
…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.
Evan-M pushed a commit to Evan-M/tire that referenced this pull request Apr 21, 2012
* upstream/master: (39 commits)
  Release 0.4.0
  [GEMS] Relaxed gem dependencies for "rest-client", "multi_json" and "bundler"
  Release 0.4.0.rc
  [karmi#218] Cleaned up the test suite for document_type in Index#bulk_store
  [karmi#218] Fixed the incorrect serialization of `document_type` in Index#bulk_store
  [FIX] Fixed displaying of Rake task usage
  [karmi#289] Update README and documentation, use the `prefix` query as the example of "unsupported" query
  [karmi#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
  [karmi#289] Cleaned up the code for "term" and "fuzzy" queries and restructured the search query test suite
  [karmi#289] Improvements to the "term" query type
  [karmi#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
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants