activegraph icon indicating copy to clipboard operation
activegraph copied to clipboard

N+1 (sort of) when iterating through a QueryProxy of mixed types

Open StephenTurley opened this issue 5 years ago • 5 comments

Given three ActiveNode classes

class A
   include Neo4j::ActiveNode
   has_one :out, :foo, type: :HAS_FOO, model_class: :Foo
end

class B < A; end
class C < A; end

Assume your DB only contains instances of B and C.. .say 5 each.

Executing A.all.map { |a| a.foo } will cause an N + 1 query for the last 5 nodes in the query proxy. I think this happens because it builds the query based off of the label of the first node in the proxy. ("B:A") It would be fine if we had an instance of A at the start of the query, but for my solution we treat A like an abstract class.

I'd prefer that the query was built using the label A. Seems like that should happen when the QueryProxy is constructed via sending :all to A

Runtime information:

Neo4j database version: 3.2.7-9 neo4j gem version: 8.1.5 neo4j-core gem version: 7.0.9

StephenTurley avatar Aug 08 '18 14:08 StephenTurley

Looks like the label is being set when the AssociationProxy is created by calling foo

StephenTurley avatar Aug 08 '18 19:08 StephenTurley

Have you tried A.all.with_associations(:foo).to_a.map(&:foo) ? I don't immediately remember if A.all returns an association proxy or not, which I think is a requirement for using with_associations.

jorroll avatar Aug 11 '18 21:08 jorroll

~This still has the same issue. all returns a QueryProxy.. not sure if that matters.~

Looks like we had a monkey patch that broke with_associations. with_associations does eagerly load the association and avoids this issue. I'm going to explore using this to prefetch the associations before we send the model to our serializer.

StephenTurley avatar Aug 13 '18 13:08 StephenTurley

Seems like calling all on a class returns instances of everything and caches them in a query_proxy. Then calling map iterates over each instance, so the resulting association_proxy is built from that instance's class, not the original class where all was invoked. https://github.com/neo4jrb/neo4j/blob/master/lib/neo4j/active_node/query/query_proxy_enumerable.rb#L20

Perhaps a workaround would involve overriding the internal state of each instance such that the resulting label is always A?

If this were ActiveRecord I think you could do something like and avoid it: A.all.map { |a| a.becomes(A); a.foo }

Is there an ActiveNode equivalent?

StephenTurley avatar Aug 13 '18 14:08 StephenTurley

I'm not familiar with ActiveRecord's a.becomes(A) method, but I don't think there's an ActiveNode equivalent.

You might need to write this query using cypher. i.e.

A.query_as(:a).match("(a)-[:FOO]->(foo)")

If you know that the result of that query will always be an instance of the ActiveNode model Foo, then you can cast the query proxy to an association proxy using proxy_as() like so:

foo = A.query_as(:a).match("(a)-[:FOO]->(foo)").proxy_as(Foo, :foo)

# neo4jrb now knows that `foo` is an instance of `Foo`, and you can chain using
# association proxy methods normally. i.e.

foo.friends # assuming foo has a "has_many :out, :friends" association

edit: you might need to do A.all.query_as(:a), rather than A.query_as(:a).

jorroll avatar Aug 13 '18 17:08 jorroll