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

Added a NOW() helper #372

Merged
merged 1 commit into from Jul 18, 2014

Conversation

Projects
None yet
3 participants
@voxpelli
Contributor

voxpelli commented Jul 14, 2014

After some IRC-discussion with @tgriesser, here's a pull request for solving #310.

It adds a now() method to the database client so one can do knex.client.now() as a shortcut for knex.raw('NOW()') and then get the database specific representation of NOW() returned – which means better interoperability.

Includes one DB-specific override, that was brought up as an example in #310, for SQLite3 that replaces NOW() with date('now') instead.

@tkellen

This comment has been minimized.

Collaborator

tkellen commented Jul 14, 2014

CURRENT_TIMESTAMP, CURRENT_TIME and CURRENT_DATE are interoperable across postgresql, mysql, and sqlite3.

@tkellen

This comment has been minimized.

Collaborator

tkellen commented Jul 14, 2014

Actually, the aforementioned aliases are all ANSI SQL standards, so they should work across all databases which support the spec.

@voxpelli

This comment has been minimized.

Contributor

voxpelli commented Jul 14, 2014

@tkellen: Ok, updated accordingly

@tgriesser

This comment has been minimized.

Owner

tgriesser commented Jul 14, 2014

So is the main driver of this for schema creation, or being able to use a cross-database method for NOW() in queries?

If it's only for schema creation, it'd probably be cleaner to check the type of the column being created and then allowing a whitelisted set of defaultTo props for each database.

If it's for use anywhere, then it might be cooler to go all-out and do something like knex.date or knex.time or maybe a knex.moment namespace which implemented a clean interface for at least the ANSI standard date/time methods:

knex.moment.now()
knex.moment.currentDate()
knex.moment.currentTime()

...and then this could serve as the foundation for normalizing behavior for common date helpers cross-db:

http://www.postgresql.org/docs/8.2/static/functions-datetime.html
http://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html
http://www.sqlite.org/cvstrac/wiki?p=DateAndTimeFunctions

Thoughts?

@voxpelli

This comment has been minimized.

Contributor

voxpelli commented Jul 14, 2014

Main driver for me is for use in queries – in WHERE clauses and as values in UPDATE and INSERT.

I like the all-out approach – my initial proposal, voxpelli@1f3642b, included a mechanism for extending this to not just date helpers, but to all kind of simple helpers, so that one only has to use knex.raw() for the most complex cases and can use these methods for ordinary simple methods.

To eg. have a mechanism for expose math and string functions through as well could be useful. Possible variations could look like:

knex.function.now()
knex.function.random()
knex.function.floor('table.column')
knex.function.lower('table.column + ?', ['FooBar'])
@tgriesser

This comment has been minimized.

Owner

tgriesser commented Jul 14, 2014

Okay, now that I think about it, this is actually a pretty neat approach. I think I was initially put off by the verbosity of it, but maybe if we did knex.fn.now() it'd look pretty nice. This would also probably clean up a bit of the query compilation for the aggregate method stuff if we just did sum / min / max from here.

Also, I just pushed a commit for using getters for lazy-loading the migrate / schema stuff more cleanly. Want to just follow that pattern for the function piece and re-open the PR with the original approach, and we can any discuss implementation details from there.

@voxpelli

This comment has been minimized.

Contributor

voxpelli commented Jul 14, 2014

@tgriesser: Rebased my original commit, adapted it to both your new lazy-loading as well as to the initial feedback in here from @tkellen and updated this pull request with it.

@tgriesser

This comment has been minimized.

Owner

tgriesser commented Jul 16, 2014

Cool, want to add a minimal passing test case and I'll go ahead and merge.

Added a function helper collection
To make it possible for individual dialects to provide their own variant of common SQL-functions like NOW()
@voxpelli

This comment has been minimized.

Contributor

voxpelli commented Jul 18, 2014

@tgriesser: done!

tgriesser added a commit that referenced this pull request Jul 18, 2014

@tgriesser tgriesser merged commit f91eb95 into tgriesser:master Jul 18, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@bendrucker bendrucker referenced this pull request Jul 18, 2014

Closed

hastimestamps and now() #421

@tgriesser tgriesser referenced this pull request Sep 2, 2014

Closed

Changelog for 0.7 #460

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