ianwhite / orm_adapter

Provides a single point of entry for using basic features of ruby ORMs
MIT License
194 stars 76 forks source link

support for includes #44

Open timoschilling opened 9 years ago

timoschilling commented 9 years ago

ActiveRecord and Mongoid supports includes to eager load relations, I'm not sure about the other ORM's. Should orm_adapter support includes? I think yes. If there are more :+1: for this, I will provide a PR.

wafcio commented 9 years ago

includes exist in relational database. mongoid have many feature which allow easy use NoSQL database. MongoMapper is only for MongoDB and it doesn't support includes. So includes will not work for all ORM's.

BTW includes working like call new query so you can do it manual.

timoschilling commented 9 years ago

@wafcio I know how includes works.

The question is, should orm_adapter support includes? And have methods like includes? or support_includes?, to let the developer check that includes are supported.

The background of my question is: I'm a maintainer of @activeadmin and I'm thinking about using orm_adapter in ActiveAdmin. ActiveAdmin supports ActiveRecords::Base#includes and Mongoid::Document#includes. ActiveAdmin is planing to be ORM agnostic, orm_adapter could be a good solution.

wafcio commented 9 years ago

@timoschilling I only said that not all ORM support join and I think orm_adapter use methods which is supported by all ORM.

You wrote: "ActiveAdmin is planing to be ORM agnostic, orm_adapter could be a good solution." this mean that orm_adapter should be ORM agnostic too (in user usage), method support_includes? in my opinion is not ORM agnostic, because it will depend on used ORM, and then you should implement two ways in your app, when support_includes? return true and when support_includes? return false. Now you can assume that support_includes? return false.

I understand that includes will be helpful in your project, but includes is not part of NoSQL which orm_adapter support. Maybe I don't understand your point of view, and don't see includes method advantages.

timoschilling commented 9 years ago

I think includes are not limited to SQL, NoSQL can have includes too, see mongoid.

I think the question is, should orm_adapter only have api for methods that every orm has? create, get and delete should provided by almost every each DB, but not each DB support a find api for example.

My idea is, that orm_adapter has a "must have" and a "optional" api. And the developer need's to care about it. For the ActiveAdmin example, we can care about to have a way with and without includes.

I can understand if you say that in not the way orm_adapter should go! But I hope you think about it, bevor you say yes or no to it.

wafcio commented 9 years ago

Are you joking with includes in NoSQL ? mongoid have many extra feature which not belong to NoSQL, see mongo_mapper or even read about join in NoSQL databases - it doesn't exists. Even in readme author have written: "ORM Adapter will support only basic methods, as get, find_first, create! and so forth. It is not ORM Adapter's goal to support different query constructions, handle table joins, etc." ok, joins is not the same as includes (I know different). But i think @ianwhite should give his opinion.

timoschilling commented 9 years ago

Are you joking with includes in NoSQL?

No!

Have you ever read the implementation of includes in ActiveRecord or Mongoid? includes don't need joins (AR don't use joins for example). Nearly every ORM can have includes and nearly every database can have includes by a ORM. The only thing that a database need's is the ability to have attributes on the value or on the key and possibility to query on them.

This DB's can do it:

Tell me a Database that can't do that.

timoschilling commented 9 years ago

Every database that's supports find_all (with more then eq id) should support includes too.

wafcio commented 9 years ago

this talk give nothing valuelable information. I know that AR don't use joins and AR use includes. But i think orm_adapter is somethink like repository. includes is on ORM not in DB. I think you cannot say that database support or don't support includes, because it is ORM part. includes is somethink like additional call with data mapping which you can do it manual.

And find_all is ORM call, not database part. But as I said @timoschilling should decide, or you can add includes in pull request.

timoschilling commented 9 years ago

How dose this gem is named? orm_adaper I think! For what does ORM in orm_adaper stand for, for the same ORM that could have includes you talking about?

wafcio commented 9 years ago

This is only name, but this gem like more like repository than ORM