Skip to content

Commit

Permalink
feat: add undefined columns to undefined binding(s) error (#3425)
Browse files Browse the repository at this point in the history
  • Loading branch information
KristjanTammekivi authored and kibertoad committed Sep 4, 2019
1 parent 75ac92f commit 4ade989
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 46 deletions.
27 changes: 25 additions & 2 deletions lib/helpers.js
Expand Up @@ -36,12 +36,12 @@ function containsUndefined(mixed) {
if (isArray(mixed)) {
for (let i = 0; i < mixed.length; i++) {
if (argContainsUndefined) break;
argContainsUndefined = this.containsUndefined(mixed[i]);
argContainsUndefined = containsUndefined(mixed[i]);
}
} else if (isPlainObject(mixed)) {
Object.keys(mixed).forEach((key) => {
if (!argContainsUndefined) {
argContainsUndefined = this.containsUndefined(mixed[key]);
argContainsUndefined = containsUndefined(mixed[key]);
}
});
} else {
Expand All @@ -51,6 +51,28 @@ function containsUndefined(mixed) {
return argContainsUndefined;
}

function getUndefinedIndices(mixed) {
const indices = [];

if (Array.isArray(mixed)) {
mixed.forEach((item, index) => {
if (containsUndefined(item)) {
indices.push(index);
}
});
} else if (isPlainObject(mixed)) {
Object.keys(mixed).forEach((key) => {
if (containsUndefined(mixed[key])) {
indices.push(key);
}
})
} else {
indices.push(0);
}

return indices;
}

function addQueryContext(Target) {
// Stores or returns (if called with no arguments) context passed to
// wrapIdentifier and postProcessResponse hooks
Expand All @@ -72,4 +94,5 @@ module.exports = {
containsUndefined,
normalizeArr,
resolveClientNameWithAliases,
getUndefinedIndices,
};
4 changes: 3 additions & 1 deletion lib/query/compiler.js
Expand Up @@ -58,6 +58,7 @@ assign(QueryCompiler.prototype, {
// Collapse the builder into a single object
toSQL(method, tz) {
this._undefinedInWhereClause = false;
this.undefinedBindingsInfo = [];

method = method || this.method;
const val = this[method]() || '';
Expand Down Expand Up @@ -99,7 +100,7 @@ assign(QueryCompiler.prototype, {
debugBindings(query.bindings);
throw new Error(
`Undefined binding(s) detected when compiling ` +
`${method.toUpperCase()} query: ${query.sql}`
`${method.toUpperCase()}. Undefined column(s): [${this.undefinedBindingsInfo.join(', ')}] query: ${query.sql}`
);
}

Expand Down Expand Up @@ -372,6 +373,7 @@ assign(QueryCompiler.prototype, {
Object.prototype.hasOwnProperty.call(stmt, 'value') &&
helpers.containsUndefined(stmt.value)
) {
this.undefinedBindingsInfo.push(stmt.column)
this._undefinedInWhereClause = true;
}
const val = this[stmt.type](stmt);
Expand Down
3 changes: 2 additions & 1 deletion lib/raw.js
Expand Up @@ -104,9 +104,10 @@ assign(Raw.prototype, {

obj.bindings = obj.bindings || [];
if (helpers.containsUndefined(obj.bindings)) {
const undefinedBindingIndices = helpers.getUndefinedIndices(this.bindings);
debugBindings(obj.bindings);
throw new Error(
`Undefined binding(s) detected when compiling RAW query: ` + obj.sql
`Undefined binding(s) detected for keys [${undefinedBindingIndices}] when compiling RAW query: ${obj.sql}`
);
}

Expand Down
108 changes: 66 additions & 42 deletions test/unit/query/builder.js
Expand Up @@ -8562,40 +8562,64 @@ describe('QueryBuilder', function() {

it('Any undefined binding in a SELECT query should throw an error', function() {
var qbuilders = [
qb()
.from('accounts')
.where({ Login: void 0 })
.select(),
qb()
.from('accounts')
.where('Login', void 0)
.select(),
qb()
.from('accounts')
.where('Login', '>=', void 0)
.select(),
qb()
.from('accounts')
.whereIn('Login', ['test', 'val', void 0])
.select(),
qb()
.from('accounts')
.where({ Login: ['1', '2', '3', void 0] }),
qb()
.from('accounts')
.where({ Login: { Test: '123', Value: void 0 } }),
qb()
.from('accounts')
.where({ Login: ['1', ['2', [void 0]]] }),
qb()
.from('accounts')
.update({ test: '1', test2: void 0 })
.where({ abc: 'test', cba: void 0 }),
{
builder: qb()
.from('accounts')
.where({ Login: void 0 })
.select(),
undefinedColumns: ['Login']
},
{
builder: qb()
.from('accounts')
.where('Login', void 0)
.select(),
undefinedColumns: ['Login']
},
{
builder: qb()
.from('accounts')
.where('Login', '>=', void 0)
.select(),
undefinedColumns: ['Login']
},
{
builder: qb()
.from('accounts')
.whereIn('Login', ['test', 'val', void 0])
.select(),
undefinedColumns: ['Login']
},
{
builder: qb()
.from('accounts')
.where({ Login: ['1', '2', '3', void 0] }),
undefinedColumns: ['Login']
},
{
builder: qb()
.from('accounts')
.where({ Login: { Test: '123', Value: void 0 } }),
undefinedColumns: ['Login']
},
{
builder: qb()
.from('accounts')
.where({ Login: ['1', ['2', [void 0]]] }),
undefinedColumns: ['Login']
},
{
builder: qb()
.from('accounts')
.update({ test: '1', test2: void 0 })
.where({ abc: 'test', cba: void 0 }),
undefinedColumns: ['cba']
}
];
qbuilders.forEach(function(qbuilder) {
qbuilders.forEach(function({ builder, undefinedColumns}) {
try {
//Must be present, but makes no difference since it throws.
testsql(qbuilder, {
testsql(builder, {
mysql: {
sql: '',
bindings: [],
Expand Down Expand Up @@ -8624,31 +8648,31 @@ describe('QueryBuilder', function() {
} catch (error) {
expect(error.message).to.contain(
'Undefined binding(s) detected when compiling ' +
qbuilder._method.toUpperCase() +
' query:'
builder._method.toUpperCase() +
`. Undefined column(s): [${undefinedColumns.join(', ')}] query:`
); //This test is not for asserting correct queries
}
});
});

it('Any undefined binding in a RAW query should throw an error', function() {
var expectedErrorMessageContains =
'Undefined binding(s) detected when compiling RAW query:'; //This test is not for asserting correct queries
var raws = [
raw('?', [undefined]),
raw(':col = :value', { col: 'test', value: void 0 }),
raw('? = ?', ['test', void 0]),
raw('? = ?', ['test', { test: void 0 }]),
raw('?', [['test', void 0]]),
{ query: raw('?', [undefined]), undefinedIndices: [0] },
{ query: raw(':col = :value', { col: 'test', value: void 0 }), undefinedIndices: ['value'] },
{ query: raw('? = ?', ['test', void 0]), undefinedIndices: [1] },
{ query: raw('? = ?', ['test', { test: void 0 }]), undefinedIndices: [1] },
{ query: raw('?', [['test', void 0]]), undefinedIndices: [0] },
];
raws.forEach(function(raw) {
raws.forEach(function({ query, undefinedIndices }) {
try {
raw = raw.toSQL();
query.toSQL();
expect(true).to.equal(
false,
'Expected to throw error in compilation about undefined bindings.'
);
} catch (error) {
var expectedErrorMessageContains =
`Undefined binding(s) detected for keys [${undefinedIndices.join(', ')}] when compiling RAW query:`;
expect(error.message).to.contain(expectedErrorMessageContains); //This test is not for asserting correct queries
}
});
Expand Down

0 comments on commit 4ade989

Please sign in to comment.