Skip to content

Commit

Permalink
Changed implementation of replacing undefined with DEFAULT already in…
Browse files Browse the repository at this point in the history 
… query compiler for every dialect.
  • Loading branch information
@elhigu
elhigu committed May 12, 2016
1 parent 9fab58c commit 9114bdd
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 55 deletions.
6 changes: 2 additions & 4 deletions src/client.js
View file
Expand Up @@ -22,7 +22,7 @@ var inherits = require('inherits')
var EventEmitter = require('events').EventEmitter
var SqlString = require('./query/string')

import {assign, uniqueId, cloneDeep, map} from 'lodash'
import {assign, uniqueId, cloneDeep} from 'lodash'

var debug = require('debug')('knex:client')
var debugQuery = require('debug')('knex:query')
Expand Down Expand Up @@ -141,9 +141,7 @@ assign(Client.prototype, {
},

prepBindings: function(bindings) {
return map(bindings, (binding) => {
return binding === undefined ? this.valueForUndefined : binding
});
return bindings;
},

wrapIdentifier: function(value) {
Expand Down
9 changes: 0 additions & 9 deletions src/dialects/mssql/index.js
View file
Expand Up @@ -85,15 +85,6 @@ assign(Client_MSSQL.prototype, {
})
},

prepBindings: function(bindings) {
return map(bindings, (value) => {
if (value === undefined) {
return this.valueForUndefined
}
return value
})
},

// Grab a connection, run the query via the MSSQL streaming interface,
// and pass that through to the stream we've sent back to the client.
_stream: function(connection, obj, stream, options) {
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/mssql/query/compiler.js
View file
Expand Up @@ -44,7 +44,7 @@ assign(QueryCompiler_MSSQL.prototype, {
var i = -1
while (++i < insertData.values.length) {
if (i !== 0) sql += '), ('
sql += this.formatter.parameterize(insertData.values[i])
sql += this.formatter.parameterize(insertData.values[i], this.client.valueForUndefined)
}
sql += ')';
} else if (insertValues.length === 1 && insertValues[0]) {
Expand Down
3 changes: 0 additions & 3 deletions src/dialects/oracle/index.js
View file
Expand Up @@ -63,9 +63,6 @@ assign(Client_Oracle.prototype, {
else if (Buffer.isBuffer(value)) {
return SqlString.bufferToString(value)
}
else if (value === undefined) {
return this.valueForUndefined
}
return value
})
},
Expand Down
11 changes: 7 additions & 4 deletions src/dialects/oracle/query/compiler.js
View file
Expand Up @@ -60,7 +60,7 @@ assign(QueryCompiler_Oracle.prototype, {
sql.sql = 'begin ' +
map(insertData.values, (value) => {
var returningHelper;
var parameterizedValues = !insertDefaultsOnly ? this.formatter.parameterize(value) : '';
var parameterizedValues = !insertDefaultsOnly ? this.formatter.parameterize(value, this.client.valueForUndefined) : '';
var returningValues = Array.isArray(returning) ? returning : [returning];
var subSql = 'insert into ' + this.tableName + ' ';

Expand All @@ -79,11 +79,14 @@ assign(QueryCompiler_Oracle.prototype, {

// pre bind position because subSql is an execute immediate parameter
// later position binding will only convert the ? params

subSql = this.formatter.client.positionBindings(subSql);

var parameterizedValuesWithoutDefault = parameterizedValues.replace('DEFAULT, ', '').replace(', DEFAULT', '');
return 'execute immediate \'' + subSql.replace(/'/g, "''") +
((parameterizedValues || returning) ? '\' using ' : '') +
parameterizedValues +
((parameterizedValues && returning) ? ', ' : '') +
((parameterizedValuesWithoutDefault || returning) ? '\' using ' : '') +
parameterizedValuesWithoutDefault +
((parameterizedValuesWithoutDefault && returning) ? ', ' : '') +
(returning ? 'out ?' : '') + ';';
}).join(' ') +
'end;';
Expand Down
5 changes: 1 addition & 4 deletions src/dialects/postgres/utils.js
View file
Expand Up @@ -34,7 +34,7 @@ var arrayString;
// to their 'raw' counterparts for use as a postgres parameter
// note: you can override this function to provide your own conversion mechanism
// for complex types, etc...
var prepareValue = function (val, seen, valueForUndefined) {
var prepareValue = function (val, seen /*, valueForUndefined*/) {
if (val instanceof Buffer) {
return val;
}
Expand All @@ -47,9 +47,6 @@ var prepareValue = function (val, seen, valueForUndefined) {
if (val === null) {
return null;
}
if (val === undefined) {
return valueForUndefined;

This comment has been minimized.

Sign in to view

Copy link
@villelahdenvuo

villelahdenvuo May 19, 2016

Contributor

@elhigu This caused #1423 Because the above if used to have || val === undefined

This comment has been minimized.

Sign in to view

Copy link
@elhigu

elhigu May 19, 2016 •
edited

Author Member

Yep, I know. But having || val === undefined there is wrong way of handling this problem :) Pushing real fix in a moment. In the end I found that client.valueForUndefined to transacting client...
}
if (typeof val === 'object') {
return prepareObject(val, seen);
}
Expand Down
10 changes: 0 additions & 10 deletions src/dialects/sqlite3/index.js
View file
Expand Up @@ -110,16 +110,6 @@ assign(Client_SQLite3.prototype, {
})
},

prepBindings: function(bindings) {
return map(bindings, (binding) => {
if (binding === undefined && this.valueForUndefined !== null) {
throw new TypeError("`sqlite` does not support inserting default values. Specify values explicitly or use the `useNullAsDefault` config flag. (see docs http://knexjs.org/#Builder-insert).");
} else {
return binding
}
});
},

// Ensures the response is returned in the same format as other clients.
processResponse: function(obj, runner) {
var ctx = obj.context;
Expand Down
14 changes: 12 additions & 2 deletions src/dialects/sqlite3/query/compiler.js
View file
Expand Up @@ -3,7 +3,7 @@

var inherits = require('inherits')
var QueryCompiler = require('../../../query/compiler')
import {assign, isEmpty, isString, reduce} from 'lodash'
import {assign, isEmpty, isString, reduce, each} from 'lodash'

function QueryCompiler_SQLite3(client, builder) {
QueryCompiler.call(this, client, builder)
Expand Down Expand Up @@ -47,15 +47,25 @@ assign(QueryCompiler_SQLite3.prototype, {

sql += '(' + this.formatter.columnize(insertData.columns) + ')'

// backwards compatible error
each(insertData.values, bindings => {
each(bindings, binding => {
if (binding === undefined && this.client.valueForUndefined !== null) {
throw new TypeError("`sqlite` does not support inserting default values. Specify values explicitly or use the `useNullAsDefault` config flag. (see docs http://knexjs.org/#Builder-insert).");
}
});
});

if (insertData.values.length === 1) {
return sql + ' values (' + this.formatter.parameterize(insertData.values[0]) + ')'
return sql + ' values (' + this.formatter.parameterize(insertData.values[0], this.client.valueForUndefined) + ')'
}

var blocks = []
var i = -1
while (++i < insertData.values.length) {
var i2 = -1, block = blocks[i] = []
var current = insertData.values[i]
current = current === undefined ? this.client.valueForUndefined : current
while (++i2 < insertData.columns.length) {
block.push(this.formatter.alias(
this.formatter.parameter(current[i2]),
Expand Down
27 changes: 9 additions & 18 deletions test/unit/query/builder.js
View file
Expand Up @@ -28,15 +28,6 @@ var clientsWithNullAsDefault = {
default: new Client(useNullAsDefaultConfig)
}

var valuesForUndefined = {
mysql: clients.mysql.valueForUndefined,
sqlite3: clients.sqlite3.valueForUndefined,
oracle: clients.oracle.valueForUndefined,
postgres: clients.postgres.valueForUndefined,
mssql: clients.mssql.valueForUndefined,
default: clients.default.valueForUndefined
};

function qb() {
return clients.default.queryBuilder()
}
Expand Down Expand Up @@ -1831,7 +1822,7 @@ describe("QueryBuilder", function() {
it("multiple inserts with partly undefined keys", function() {
testquery(qb().from('users').insert([{email: 'foo', name: 'taylor'}, {name: 'dayle'}]), {
mysql: "insert into `users` (`email`, `name`) values ('foo', 'taylor'), (DEFAULT, 'dayle')",
oracle: 'begin execute immediate \'insert into "users" ("email", "name") values (:1, :2)\' using \'foo\', \'taylor\'; execute immediate \'insert into "users" ("email", "name") values (:1, :2)\' using DEFAULT, \'dayle\';end;',
oracle: 'begin execute immediate \'insert into "users" ("email", "name") values (:1, :2)\' using \'foo\', \'taylor\'; execute immediate \'insert into "users" ("email", "name") values (DEFAULT, :1)\' using \'dayle\';end;',
mssql: "insert into [users] ([email], [name]) values ('foo', 'taylor'), (DEFAULT, 'dayle')",
default: 'insert into "users" ("email", "name") values (\'foo\', \'taylor\'), (DEFAULT, \'dayle\')'
});
Expand Down Expand Up @@ -1956,12 +1947,12 @@ describe("QueryBuilder", function() {
bindings: [1, undefined, undefined, undefined, 2, undefined, 2, undefined, 3]
},
oracle: {
sql: "begin execute immediate 'insert into \"table\" (\"a\", \"b\", \"c\") values (:1, :2, :3)' using ?, ?, ?; execute immediate 'insert into \"table\" (\"a\", \"b\", \"c\") values (:1, :2, :3)' using ?, ?, ?; execute immediate 'insert into \"table\" (\"a\", \"b\", \"c\") values (:1, :2, :3)' using ?, ?, ?;end;",
bindings: [1, valuesForUndefined.oracle, valuesForUndefined.oracle, valuesForUndefined.oracle, 2, valuesForUndefined.oracle, 2, valuesForUndefined.oracle, 3]
sql: "begin execute immediate 'insert into \"table\" (\"a\", \"b\", \"c\") values (:1, DEFAULT, DEFAULT)' using ?; execute immediate 'insert into \"table\" (\"a\", \"b\", \"c\") values (DEFAULT, :1, DEFAULT)' using ?; execute immediate 'insert into \"table\" (\"a\", \"b\", \"c\") values (:1, DEFAULT, :2)' using ?, ?;end;",
bindings: [1, 2, 2, 3]
},
mssql: {
sql: 'insert into [table] ([a], [b], [c]) values (?, ?, ?), (?, ?, ?), (?, ?, ?)',
bindings: [1, valuesForUndefined.mssql, valuesForUndefined.mssql, valuesForUndefined.mssql, 2, valuesForUndefined.mssql, 2, valuesForUndefined.mssql, 3]
sql: 'insert into [table] ([a], [b], [c]) values (?, DEFAULT, DEFAULT), (DEFAULT, ?, DEFAULT), (?, DEFAULT, ?)',
bindings: [1, 2, 2, 3]
},
default: {
sql: 'insert into "table" ("a", "b", "c") values (?, DEFAULT, DEFAULT), (DEFAULT, ?, DEFAULT), (?, DEFAULT, ?)',
Expand Down Expand Up @@ -3308,12 +3299,12 @@ describe("QueryBuilder", function() {
bindings: ['test', 1, 'none']
},
oracle: {
sql: 'begin execute immediate \'insert into "users" ("id", "name", "occupation") values (:1, :2, :3)\' using ?, ?, ?; execute immediate \'insert into "users" ("id", "name", "occupation") values (:1, :2, :3)\' using ?, ?, ?;end;',
bindings: [valuesForUndefined.oracle, 'test', valuesForUndefined.oracle, 1, valuesForUndefined.oracle, 'none']
sql: 'begin execute immediate \'insert into "users" ("id", "name", "occupation") values (DEFAULT, :1, DEFAULT)\' using ?; execute immediate \'insert into "users" ("id", "name", "occupation") values (:1, DEFAULT, :2)\' using ?, ?;end;',
bindings: ['test', 1, 'none']
},
mssql: {
sql: 'insert into [users] ([id], [name], [occupation]) values (?, ?, ?), (?, ?, ?)',
bindings: [valuesForUndefined.mssql, 'test', valuesForUndefined.mssql, 1, valuesForUndefined.mssql, 'none']
sql: 'insert into [users] ([id], [name], [occupation]) values (DEFAULT, ?, DEFAULT), (?, DEFAULT, ?)',
bindings: ['test', 1, 'none']
},
postgres: {
sql: 'insert into "users" ("id", "name", "occupation") values (DEFAULT, ?, DEFAULT), (?, DEFAULT, ?)',
Expand Down

0 comments on commit 9114bdd

Please sign in to comment.