Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Single Table Inheritance support #178

Closed
haihappen opened this Issue · 27 comments
@haihappen

My app search for links in various 3rd-party services like Delicious, Twitter … I have following base class:

class Link
  include Mongoid::Document
  include Tire::Model::Search
  include Tire::Model::Callbacks

  field :href, type: String
  field :name, type: String

  mapping do
    indexes :href, type: 'string', analyzer: 'url'
    indexes :name, type: 'string', analyzer: 'keyword', boost: 10
  end
end

and following class inherits from Link and adds two more fields:

class Link::Delicious < Link
  field :tags, type: Array
  field :time, type: Time

  mapping do
    indexes :tags, type: 'string', analyzer: 'keyword'
    indexes :time, type: 'date'
  end
end

Searches will be done via the Base class: Link.search('google.com'). Is there any chance to get this working? At the moment the (additional) fields in Link::Delicious are completely ignored by Tire/ElasticSearch.

@haihappen

Working fix:

class Link
  class << self
    def mapping_with_super(*args, &block)
      # Creating only one index
      index_name('links')
      document_type('link')

      superclass.mapping_without_super.each do |name, options|
        indexes(name, options)
      end if superclass.respond_to?(:mapping)

      mapping_without_super(args, &block)
    end
    alias_method_chain :mapping, :super
  end
end
@karmi
Owner

@haihappen: Does it make any sense to de-compose your models for searching, like this? They end up as one JSON doc anyway, right? An interesting twist would be to use different _type for each of these links within ES.

Now, this probably won't work anyway -- the index is created with proper mapping on class load, only when the index does not exist yet. Since one class will be eventually loaded first, it would "win" this race.

In near future, this behaviour will probably change, and Tire will update the mapping, and also complain when the mapping in ES and mapping declared in Tire won't match.

@rtlong

@karmi Which would be the best way to approach this? I've got two sets of STI models in my app and I can't quite figure out how to do this.

@karmi
Owner

@rtlong I'd probably extracted the index creation and mapping definition logic out of the models entirely. I'd put it in /lib, or maybe keep them in the “base” model for STI. I believe there's some page on the Wiki which has the exact for such extraction.. Ping me here or on IRC (#elasticsearch @ Freenode) if you get stuck.

@karmi karmi closed this issue from a commit
@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 closed this in d5c08fb
@nicolas2bonfils nicolas2bonfils referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@fbjork

Still unclear to me how to get STI working with Tire.

I setup mappings in the base class, and set the index_name, but it's not working as expected. The _type field gets overwritten by Tire to "ancestor/child".

@karmi
Owner

@fbjork Is it possible to paste/gist simplified model definitions? Also, are the mappings correctly saved in ES (see discussion above)?

@richarddong

Hey, guys!

and @haihappen , @karmi

I've found an EASY way to do Single Table Inheritance using Tire, in a single elasticsearch index.

class Link
  include Mongoid::Document
  include Tire::Model::Search
  include Tire::Model::Callbacks

  tire.index.add_alias 'diggs'
  # redirect any search on the index 'diggs' to the index of `Link`, namely 'links'

  field :href, type: String
  field :name, type: String

  mapping do
    indexes :href, type: 'string', analyzer: 'url'
    indexes :name, type: 'string', analyzer: 'keyword', boost: 10
  end
end
class Digg < Link

  tire.index_name 'links'  # or tire.index_name superclass.tire.index_name
  # store diggs to the index of `Link`, namely 'links'

  field :tags, type: Array
  field :time, type: Time

  mapping do
    indexes :tags, type: 'string', analyzer: 'keyword'
    indexes :time, type: 'date'
  end
end

To SEARCH ALL INHERITED CLASSES such as diggs DIRECTLY FROM links, put this in your links_controller.rb

def search
  @links = Link.search(params[:query], type: nil)
  # override `default_options = {:type => document_type, :index => index.name} on /lib/tire/model/search.rb#L66
end

To SEARCH ONLY diggs, put this in your diggs_controller.rb

def search
  @diggs = Digg.search(params[:query])
  # or equivalently
  @diggs = Link.search(params[:query], type: "digg")  # or `type: Digg.tire.document_type`
end
@allochi

@richarddong

Can't thank you enough for sharing this man!

@elfassy

I still don't get this. Using the first post as example

curl -XGET 'http://localhost:9200/links/_mapping'

{"links":{"link/delicious":{"properties": ...

Is this correct? It seems to work if i use type: nil.

tire.search(load: true, page: params[:page], type: nil) do
   ...
end

If anything, line 78 (https://github.com/karmi/tire/blob/master/lib/tire/model/naming.rb) should be (for consistency with the index name)

@document_type || klass.model_name.underscore.gsub("/","_")
@karmi
Owner

klass.model_name.underscore.gsub("/","_") would break loading your models from the database. The whole toolchain (Tire, Rails, ...) expects the class names being serialized in a certain way.

@richarddong

@karmi
Is STI for Mongoid working properly now? If not, what's the plan? thx.

@gregory

Just want to confirm that it's working like a charm with mongo.

I had SocialComments::Accounts::Google < SocialComments::Account in mongo

At first i had some issue but when watching at the logs you can see:

Tire::Search::SearchRequestFailed: 404 : {"error":"IndexMissingException[[social_comments_accounts_googles] missing]","status":404}

so just had to add an index alias as @richarddong said so in my case:

tire.index.add_alias 'social_comments_accounts_googles' 
@karmi
Owner

@gregory Cool!

I don't remember the exact issue context, but also note that you should be able to just use index_name to override the index name if you want to control it explicitely. In new Tire versions, it's possible to store multiple models in one index.

@gregory

Though i faced a "issue" trying to index embeded content in elastic. So i had Social accounts like fb and many comments for each account.
So i wanted to index those comments embeded in an account(there is not that much) and be able to search among them.
It's working fine but it's useless since elastic will return an account related to my search query but with all the comments ... so i had to extract comments from account and use has_many

It's more an architecture problem than a tire or mongoid issue, just be aware of that :)

@karmi
Owner

Yes, embedded docs / associated models / etc are always tricky to model right. Note that it's possible to retrieve just specific fields in the response.

When working with a structure like article :has_many comments, it's often necessary to use the nested mapping type for comments.

@gregory

I faced some issues with inheritance with mongo. soi come back to you guys providing my solutions; There are more quick fix than solutions but this may help you.

It seems that tire tends to overwrite the _type= method
which may conflict in a mongo context so her is the solution:
class A

  include Mongoid::Document
  include Mongoid::Timestamps
  include Tire::Model::Search
  include Tire::Model::Callbacks
  def _type=(value)
    self[:_type] = value.camelize
  end

  def self.inherited(klass)
    super
    klass.tire.index_name self.tire.index.name
  end
end

I was unable to search over all the childs in a inheritance context so here is the quick fix:
def self.search(params, options={})
options.merge!({ type: self.tire_aliases }) #if you wanna search for comments in all the chilld classes
tire.search(options) do |s|
end
end
#TYPES should be all your child classes so:
# if class B < A; end; TYPES=:b
TYPES = [:b]
def self.tire_aliases
TYPES.map(&:to_s).map{ |type| "social_comments/comments/#{type}" }
end

Hope this can help

Merry X mass

@otobrglez

Thanks @gregory for your advice. Saved me some time. :+1:

@davesouth

Thanks @richarddong for the excellent example of Mongoid/Tire inheritance.

However, mapping the index for multiple types is proving a challenge.

@karmi wrote:

@rtlong I'd probably extract the index creation and mapping definition logic out of the models entirely. I'd put it in /lib, or maybe keep them in the “base” model for STI. I believe there's some page on the Wiki which has the exact for such extraction.

Agreed. However, the wiki entry doesn't exist nor can I find any examples of mapping each _type in an index. We need mapping to look something like this (where "phone_ngram" is our custom analyzer):

contact: {
  person: {
    properties: {
      first: { type: 'string', analyzer: 'snowball' },
      last: { type: 'string', analyzer: 'snowball' },
      phones: {
        properties: {
          digits: { type: 'string', analyzer: 'phone_ngram' },
        }
      },
    }
  },
  company: {
    properties: {
      name: { type: 'string', analyzer: 'snowball' },
      phones: {
        properties: {
          digits: { type: 'string', analyzer: 'phone_ngram' },
        }
      },
    }
  }
}

Any suggestions?

@karmi
Owner

@davesouth What specific advice can I give here? You can create an index with desired settings/mappings beforehand like:

MY_MAPPINGS = {type1: ..., type2: ...}
Tire.index('myindex').create mappings: MY_MAPPINGS
@davesouth

@karmi Yes, I can get two _type 's created or I can do nested, but something fails when trying to do both. I know it's something I'm doing wrong. It's got to be a simple mistake. There's just no obvious reason for the failure.

EDIT: Moved questionable code into Gist:
https://gist.github.com/davesouth/5699181

@karmi
Owner

@davesouth Fortunately, this sounds easy .) First, a bit of advice, when somethings aloof like this, first thing to do is stick Tire.configure { logger STDERR, level: 'debug' } somewhere in the script, so you can see Elasticsearch responses.

Then, you've mixed phone_analyzer and phone_ngram for digits -- use phone_analyzer there.

@davesouth

@karmi Ah ha! That fixed it. Thank you.

I fixed the "phone_analyzer" problem before. However, with all the variations and different configurations, I must have picked up the error again. The logger tip will come in very handy.

Here is the final, working version for anyone trying to do something similar:

Tire.index('contacts').delete

Tire.index('contacts').create settings: {
  analysis: {
    analyzer: {
      phone_analyzer: { tokenizer: 'whitespace', filter: 'phone_ngram', type:'custom' }
    },
    filter: {
      phone_ngram: { type: 'nGram', min_gram: 3, max_gram: 15 }
    }
  }
},
mappings: {
  person: {
    properties: {
      id: { type: 'string', index: 'not_analyzed' },
      first: { type: 'string', analyzer: 'snowball' },
      last: { type: 'string', analyzer: 'snowball' },
      title: { type: 'string', analyzer: 'snowball' },
      phones: {
        properties: {
          digits: { type: 'string', analyzer: 'phone_analyzer' }
        }
      }
    }
  },
  company: {
    properties: {
      id: { type: 'string', index: 'not_analyzed' },
      name: { type: 'string', analyzer: 'snowball', boost: 10 },
      phones: {
        properties: {
          digits: { type: 'string', analyzer: 'phone_analyzer' }
        }
      }
    }
  }
}
@karmi
Owner

@davesouth Cool. Just remember you can stick the whole definition in a constant, or a module method, or make it dynamic and assemble via a module method, etc.

@davesouth

@karmi Yes, definitely. Especially where there will be so much duplication between the two types (phones, emails, addresses, etc).

@Bounga

@richarddong thanks bro. You saved my day. Excellent tip.

@richarddong

@Bounga :smiley: this hack still might fail..

@ankane ankane referenced this issue in ankane/searchkick
Closed

Problem with Single Table Inheritance models #30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.