Skip to content
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

Model Inheritance #763

Closed
jamilabreu opened this issue Apr 20, 2015 · 8 comments
Closed

Model Inheritance #763

jamilabreu opened this issue Apr 20, 2015 · 8 comments
Assignees

Comments

@jamilabreu
Copy link

A School model inherits from Community, and communities can be part_of other communities:

class School < Community
end

class Community
  include Neo4j::ActiveNode
  has_many :out, :part_of, model_class: Community
end

The below code only creates the relationship correctly if school.rb explicitly includes has_many :out, :part_of, model_class: Community

School.create(name: name, part_of: [ivy_league])

like this:

class School < Community
  has_many :out, :part_of, model_class: Community
end
@subvertallchris
Copy link
Contributor

This is definitely due to the code I introduced to tweak what Brian added earlier. You take a small performance hit when you mix declared properties and association names, so my PR tried to identify nodes that are not doing this to limit its impact. It needs to initialize the variable when inherited. Lemme see if I can do this quickly but if not, I can take care of it in the morning.

@subvertallchris
Copy link
Contributor

Will merge in the morning if all passes.

@jamilabreu
Copy link
Author

Cool 😄 So for best performance would you recommend not mixing properties and associations? Something like:

school = School.create(name: name)
school.part_of << ivy_league

@subvertallchris
Copy link
Contributor

Yes, definitely. When you mix associations and node properties, not only do you take a performance hit when it splits the one params hash into separate hashes, but it uses the = operator to create relationships. That results in an n+1 query against the database. It wipes out existing relationships, then it creates new ones. It's a fast process since it doesn't return anything from the DB and the query itself is very efficient, but it's still slower than <<.

None of this is really a problem in isolation but if speed is a concern, like when you're seeding a database, it can be a big issue. It also just adds up if you have other things happening within your app or database.

@subvertallchris
Copy link
Contributor

This is merged in, fixed now. Made me realize there are probably a few other bugs related to inheritance that I'm going to check now.

@jamilabreu
Copy link
Author

Working on this end 👍 thanks for the tip, btw! I'll keep poking around, see if I find anything

@jamilabreu
Copy link
Author

@subvertallchris: Came across another potential inheritance bug:

class Person
  include Neo4j::ActiveNode
  has_many :out, :communities, type: "PART_OF"
  has_many :out, :schools
end

class Community
  include Neo4j::ActiveNode
  has_many :out, :communities, type: "PART_OF"
end

class School < Community
end

Adding a person to a school via a form:

= form_for current_person do |f|
  = f.collection_select :schools, School.all, :id, :name, { prompt: true, selected: current_person.schools.map(&:id) }

correctly adds the selected schools to current_person.schools, but current_person.communities returns an empty array

UPDATE
My error was that I needed to add the type: has_many :out, :schools, type: "PART_OF"

@subvertallchris
Copy link
Contributor

Could you message me in the Gitter room? I have so many questions. gitter.im/neo4jrb/neo4j

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants