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

Improved supported data-types in min() and max() #1200

Merged
merged 8 commits into from Jan 5, 2014

Conversation

2 participants
@xdarklight
Contributor

xdarklight commented Jan 4, 2014

When I started only numeric types (int and float) were supported.
After my patches 'Date' and 'String' work as well (I added a JUnit test to prove this).

Please carefully review this as I'm not sure if my new "aggregate" function in 1504b6f is good the way it is (and I'm not sure if this should be public API/has to be documented?).

PS: I only tested sqlite so far.

xdarklight added some commits Dec 31, 2013

Introduced a aggregate() function in the DAOFactory so code can be sh…
…ared.

Also made rawSelect's data-types options more extensible and implemented
the "DATE" case.
Define the 'DECIMAL' data-type similar to how a FLOAT is defined.
This gives us the possibility to use "instanceof DECIMAL" when needed.
Also handle DECIMAL (similar to the way FLOATs are handled).
This should fix min() and max() for postgres.
Fixed 'type' property of DECIMAL.
This fixed query generation (before the column type was always DECIMAL,
even if DECIMAL(10,3) was specified).
Fixed parsing non-standard DECIMAL during min() and max().
DECIMAL() and DECIMAL(10,3) are not the same - but the type matches.
@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Jan 5, 2014

Looks good - I'm going to merge this.
We might want to look at using .find() with .fn() and .col() later on

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

Merge pull request #1200 from xdarklight/master
Improved supported data-types in min() and max()

@mickhansen mickhansen merged commit 29ced24 into sequelize:master Jan 5, 2014

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