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

add With Clause #1599

Merged
merged 6 commits into from Sep 13, 2016

Conversation

Projects
None yet
3 participants
@atiertant
Contributor

atiertant commented Jul 28, 2016

fix #716
add a with function(alias, callback|raw)

knex.with('with_alias', function() {
    this.select('*').from('books').where('author', 'Test')
}).select('*').from('with_alias')

output:
with "with_alias" as (select * from "books" where "author" = 'Test') select * from "with_alias"

}).select('*').from('withClause'), {
mssql: 'with [withClause] as (select [foo] from [users]) select * from [withClause]',
sqlite3: 'with "withClause" as (select "foo" from "users") select * from "withClause"',
postgres: 'with "withClause" as (select "foo" from "users") select * from "withClause"',

This comment has been minimized.

@elhigu

elhigu Sep 9, 2016

Collaborator

Is it intentional that many of the tests doesn't have oracle / oracledb result checks?

This comment has been minimized.

@atiertant

atiertant Sep 12, 2016

Contributor

in oracle the with clause is used in a select subquery.
this could be used in insert but could not find a way to use it with knex (raw or not...)

if (statement instanceof Raw && arguments.length >= 2) {
return this.withRaw(alias, statement, bindings);
}

This comment has been minimized.

@elhigu

elhigu Sep 9, 2016

Collaborator

I suppose that this line should never be run, since only allowed parameters are function and knex.raw. Throwing exception and adding test for that would be good to prevent undefined does not have property ... errors.

This comment has been minimized.

@atiertant

atiertant Sep 12, 2016

Contributor

are you talking about the return in line 83 ?
i could replace return by return this like other functions :
there is no other function who throw any exception in this file...
would you like me to return by:
throw new Error("invalid arguments for with()");

This comment has been minimized.

@elhigu

elhigu Sep 12, 2016

Collaborator

Yep, could be even more detailed describing that argument must be either function / raw. There are no exceptions thrown in this file currently, but actually many of the function does throw an error if called with invalid parameters.

@elhigu

This comment has been minimized.

Collaborator

elhigu commented Sep 9, 2016

Looks good. I added couple of small comments. Also would be good to add at least one integration test for this.

@elhigu elhigu merged commit 69235ae into tgriesser:master Sep 13, 2016

1 check passed

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

tgriesser added a commit that referenced this pull request Sep 13, 2016

Merge branch 'master' into 0.12-prep
* master:
  add With Clause (#1599)

tgriesser added a commit that referenced this pull request Sep 14, 2016

Merge branch 'master' into refactor
* master:
  release 0.12.0
  Remove docs, in favor of https://github.com/knex/documentation (#1666)
  Revert to generic pool (#1665)
  Fix #1619
  Fix use of const in test suite for node 0.12
  Moving bin/cli outside of src to allow install from master
  Deprecate Knex.Promise
  Simplifying internal client structure
  add With Clause (#1599)
  Simplify transaction classes
  Simplify formatter use
  Deprecate VERSION, update changelog
  Fix PG string escaping behavior (#1661)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment