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

Filters through relations #991

Merged
merged 7 commits into from Oct 15, 2013

Conversation

2 participants
@snit-ram
Contributor

snit-ram commented Oct 15, 2013

Adding support for filtering data through relationships. Good examples of this can be found in the file test/associations/multiple-level-filters.test.js

Here is the added behavior:

Task.findAll({
  where: {
    'project.user.username': 'leia'
  }
}).success(function(tasks){
  //...
});

By running this code, it sequelize will interpret the where clauses adding all the necessary joins to able to run the filter.

Hope you guys like it and consider the merging

@sdepold

This comment has been minimized.

Member

sdepold commented Oct 15, 2013

wow. need to take some further time to review the code :)

@snit-ram

This comment has been minimized.

Contributor

snit-ram commented Oct 15, 2013

I know i'ts big and can seem scary at the first look, but let me give you guys an overview.

  • Theres a lot of new functions to identify this type of filtering and to get the necessary joins
  • hashToWhereConditions, getWhereConditions and selectQuery are the mysql version with minor changes to support the new filtering.
  • I've extracted some database-specific code (like boolean value serialization, select from an array in postgres, and limit & offset) to smaller functions. Due to this change, hashToWhereConditions, getWhereConditions and selectQuery are now the same for every database and was moved to the abstract query generator. That's why you guys see a lot of deletions.

That's it. I tried to follow your code style, so it should not be that difficult to read.

The tests also help to understand the purpose of the change, and are passing in all databases.

If you guys need something from me in order to merge it, please tell me. I'll be glad to help.

@sdepold

This comment has been minimized.

Member

sdepold commented Oct 15, 2013

pretty awesome stuff :)

@sdepold sdepold merged commit 916a803 into sequelize:master Oct 15, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment