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

Adding HAVING clause support to generic query generator #1286

Merged
merged 7 commits into from Jan 24, 2014

Conversation

2 participants
@OtaK
Contributor

OtaK commented Jan 24, 2014

Hi guys, hi @sdepold,

As I needed it, I added HAVING clause support to Sequelize's generic query generator.

Do I need to provide unit tests or is it okay like that ?

HOW TO USE

As the specification says, HAVING clauses are syntactically the same as WHERE clauses.
So in your Model.findAll() for example, provide a having option in the hash.

Like this :

Model.findAll({
    ...
    having: { myField: { gt: 600 } },
    ...
}).success(...).error(...);
@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jan 24, 2014

Please do provide unit tests when introducing new functionality, else we will have no clue if we break it later on.

@OtaK

This comment has been minimized.

Contributor

OtaK commented Jan 24, 2014

Just pushed some commits with bugfixes, improvements, code style, and unit tests :)

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jan 24, 2014

Great stuff, i see you added tests to mysql query generator, mind adding to sqlite and postgres aswell? :)

Fixed a code style issue in mysql query generator unit tests
Added Postgres and SQLite unit tests
@OtaK

This comment has been minimized.

Contributor

OtaK commented Jan 24, 2014

Here it is for you :-)

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jan 24, 2014

Ah, we merged some work in the meanwhile, mind rebasing? I cant merge automatically.

OtaK added some commits Jan 24, 2014

Merge branch 'master' of github.com:sequelize/sequelize
Conflicts:
	lib/dialects/abstract/query-generator.js
@OtaK

This comment has been minimized.

Contributor

OtaK commented Jan 24, 2014

Just pushed the merge. Let's wait for Travis to complete the build.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jan 24, 2014

Sweet :)

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jan 24, 2014

Mind adding a small doc to the original description of this PR describing how to use it?

mickhansen added a commit that referenced this pull request Jan 24, 2014

Merge pull request #1286 from OtaK/master
Adding HAVING clause support to generic query generator

@mickhansen mickhansen merged commit 0cd08d9 into sequelize:master Jan 24, 2014

1 check passed

default The Travis CI build passed
Details
@OtaK

This comment has been minimized.

Contributor

OtaK commented Jan 24, 2014

@mickhansen Done. Does it look okay for you?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jan 24, 2014

@OtaK Perfect! Thank you for your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment