Eager loading of nested associations

[ben-ng]

Not ready for merge yet. First cut at this feature.

TODO:

• [x] Querying on nested associations
• [x] Sorting on nested associations

TESTS:

• [x] through -> through
• [x] through -> hasMany
• [ ] through -> belongsTo
• [x] through -> hasOne
• [ ] hasMany -> through
• [ ] hasMany -> hasMany
• [x] hasMany -> belongsTo
• [ ] hasMany -> hasOne
• [x] belongsTo -> through
• [x] belongsTo -> hasMany
• [ ] belongsTo -> belongsTo
• [ ] belongsTo -> hasOne
• [ ] hasOne -> through
• [ ] hasOne -> hasMany
• [ ] hasOne -> belongsTo
• [ ] hasOne -> hasOne

// OLD.. existing syntax
model.Thing.all({}, {includes: 'assoc1'}, function (err, data) {});
// Equiv to:
model.Thing.all({}, {includes: ['assoc1']}, function (err, data) {});
// Multiple assocs:
model.Thing.all({}, {includes: ['assoc1', 'assoc2']}, function (err, data) {});

// NEW! Nested assoc syntax:
model.Thing.all({}, {includes: {assoc1: 'assoc2'}}, function (err, data) {});
// Equiv to:
model.Thing.all({}, {includes: {assoc1: ['assoc2']}}, function (err, data) {});
// Further nesting:
model.Thing.all({}, {includes: {assoc1: {assoc2: 'assoc3'}}}, function (err, data) {});
// Equiv to:
model.Thing.all({}, {includes: {assoc1: {assoc2: ['assoc3']}}}, function (err, data) {});
// Go nuts:
model.Thing.all({}, {includes: {assoc1: {assoc2: ['assoc2.1', 'assoc2.2']}}}, function (err, data) {});

Breaking change: ordering on associations must be done by association name, not table name. Otherwise you'd get conflicts if loading two associations from the same table.

// old behavior:
model.Event.all({}, {includes: 'participants', sort: {'people.id': 'desc'}}, function (err, data) {});
// this would conflict if eager loading two assocs that lead to the `people` table:
model.Event.all({}, {includes: ['participants', 'admins'], sort: {'people.id': 'desc'}}, function (err, data) {});

// new behavior:
model.Event.all({}, {includes: 'participants', sort: {'participants.id': 'desc'}}, function (err, data) {});

implementation: The old "_sqlMetaData" methods have been replaced with the recursive variants in this file.

The "From x x" stuff and the createLeftOuterJoin method have been replaced with a recursive createFrom method.

The EventedQueryProcessor has been rewritten to support fast recursive reification. The handleRow method is no more. Instead of iterating through the columns of each row, there's a map of precomputed pluck functions that create or look up existing model instances and attach them to their rightful owners.

[mde]

This is fucking fantastic.

[ben-ng]

There are 16 inductive hypothesis that need to be proven for every permutation of the four base cases.. (hasMany/belongsTo/hasOne/through)^2

So the next order of business is writing up all the tests (:

[ben-ng]

Okay, got this working with through->through. I've added checklists of things that need to be done.

The first priority is the other 14 tests. Then I'd like to make querying and sorting work on nested associations as well.

[danfinlay]

That's incredible work, I can't wait to take this for a test drive tomorrow!

[ben-ng]

@flyswatter thanks! let me know if any relations break and i'll tackle those tests next.

i've only written 2 of the 16 we need, so there's plenty of opportunity to break things.

[danfinlay]

Sounds like a good time to start experimenting with test generating code ;)

[mde]

@flyswatter Good call, +1 on beginning to automate the automation. :D

[ben-ng]

I feel like the test generator for this would be sufficiently complicated that it would need its own tests..

[techwraith]

We should write a test generator to generate tests for the test generator :P

On Sun, Apr 13, 2014 at 9:29 PM, Ben notifications@github.com wrote:

I feel like the test generator for this would be sufficiently complicated that it would need its own tests..

Reply to this email directly or view it on GitHubhttps://github.com/geddy/model/pull/184#issuecomment-40332575 .

[artzstudio]

Are there (or do you know of other) plans to implement the "include" directive in other adapters than SQL? I'm quickly realizing I need this feature if I'm going to continue to leverage Model and wanted to see if it was worth implementing myself.

[mde]

Eager-fetch of associations (the include) is implemented using SQL JOIN statements. This is not possible in non-relational DBs. (See this SO question for some context: http://stackoverflow.com/questions/2350495/how-do-i-perform-the-sql-join-equivalent-in-mongodb) The bottom line is that associations are relational -- they involve relationships in your data. The NoSQL adapters are explicitly non-relational. You can hack around this limitation if you want to, but the only real solutions are either denormalizing your data (making redundant copies which then all have to be kept in sync), or doing a large number of queries to assemble the data, which is extremely slow and resource-intensive. If you have relational data, using a RDBMS is probably a better choice for your app.

[artzstudio]

Makes sense. It would be pretty neat if this were a feature of Model, i.e. optionally storing the denormalized associations for you on the object and smartly keeping them in sync. Just pondering, thanks for the reply.

[danfinlay]

Hey @ben-ng I know I'd had trouble with this before, but I just pulled your latest and tried it, and it worked! The hasMany/belongsTo association can be eagerly loaded, and I see it works with a single request to Postgres, it's beautiful! Absolutely beautiful. Thank you so much.

[ben-ng]

@flyswatter thats awesome news! I still have one bug to fix and this will be ready for merge.

[mde]

So exciting. :)

[danfinlay]

Found one bug to test for, on your current branch @ben-ng, querying for an array of IDs returns several instances of each result.

[ben-ng]

@flyswatter hm, that's interesting. if you can add a failing test to the branch i'll take a look tonight.

[ben-ng]

@mde can you confirm that this is a bad test? 99f1277a54e55ee2f821ee58574bf1368b937b09

Seems like querying inside an object should use nested objects, not the eager assn dot syntax. I didn't even know we supported that, did someone add it to mongo or something?

[mde]

I don't remember it, but it looks like we added it for Mongo here: 186f53488f4ab053bdf081b9eb2c9b9efaeba1c7

[ben-ng]

Oh. Hmmmmmmm. We should probably add tests for that and figure out what syntax we want for that kind of query. This PR should be good to go though. I didn't get to write all the tests I wanted, but I'm pretty sure that they would hit the same code paths anyway.

[mde]

I'll give you the honor of closing this. Awesome work, man!

[ben-ng]

BOOM!