Skip to content

Commit f12a5c6

Browse files
ojourmelojourmel
committed
Refactor mssql dialect as per pull 2do2go#26 feedback
This commit refactors the `mssql` dialect as per the feedback from @artzhookov on 2do2go#26. - Include all test files in gulp lint tasks - Move mssql `value` block override to blocks.js - Update mssql `limit` block to use `_.isUndefined` - Clean up minor mssql sql letter case and format - Set the `mssql` `identifier` `prefix` and `suffix` This commit also introduces two changes to the `base` dialect. - Refactor the `base `insert:values` `block` function to clean up the logic - Refactor the `mssql` `inserted:values` `block` function to match the `base` function, and also include `returning` params, as it is needed. (mssql puts `output` keyword before `values` keyword) - Update `buildTemplate` to include an edge case where a `buildBlock` function does not return any value. This prevents unnecessary whitespace in the final query string. These changes are not strictly necessary. The `mssql` `insert:values` `block` could use the same structure as the original `base` `block`, and unnecessary whitespace has no issues in terms of functionality, simply ascetics. This commit refactors the `mssql` tests as per the changes made. Test coverage and layout is not ideal, but functional.
1 parent 6bd2f6a commit f12a5c6

File tree

7 files changed

+92
-80
lines changed

7 files changed

+92
-80
lines changed

‎gulpfile.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ gulp.task('coverage', function(callback) {
1818
.pipe(istanbul())
1919
.pipe(istanbul.hookRequire())
2020
.on('finish', function() {
21-
gulp.src(['./tests/*.js'], {read: false})
21+
gulp.src(['./tests/**/*.js'], {read: false})
2222
.pipe(mocha({
2323
reporter: 'spec',
2424
bail: true

‎lib/dialects/base/blocks.js

+10-7
Original file line numberDiff line numberDiff line change
@@ -333,10 +333,11 @@ module.exports = function(dialect) {
333333

334334
dialect.blocks.add('insert:values', function(params) {
335335
var values = params.values;
336+
var fields = params.fields;
336337

337338
if (!_.isArray(values)) values = [values];
338339

339-
var fields = params.fields || _(values)
340+
fields = fields || _(values)
340341
.chain()
341342
.map(function(row) {
342343
return _(row).keys();
@@ -345,13 +346,15 @@ module.exports = function(dialect) {
345346
.uniq()
346347
.value();
347348

349+
values = _(values).map(function(row) {
350+
return _(fields).map(function(field) {
351+
return dialect.buildBlock('value', {value: row[field]});
352+
});
353+
});
354+
348355
return dialect.buildTemplate('insertValues', {
349-
fields: fields,
350-
values: _(values).map(function(row) {
351-
return _(fields).map(function(field) {
352-
return dialect.buildBlock('value', {value: row[field]});
353-
});
354-
})
356+
values: values,
357+
fields: fields
355358
});
356359
});
357360

‎lib/dialects/base/index.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,8 @@ Dialect.prototype.buildTemplate = function(type, params) {
347347
return '';
348348
} else {
349349
if (self.blocks.has(type + ':' + block)) block = type + ':' + block;
350-
return self.buildBlock(block, params) + space;
350+
var result = self.buildBlock(block, params);
351+
return result === '' ? result : result + space;
351352
}
352353
}).trim();
353354
};

‎lib/dialects/mssql/blocks.js

+58-27
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,52 @@
33
var _ = require('underscore');
44

55
module.exports = function(dialect) {
6+
var parentValueBlock = dialect.blocks.get('value');
7+
8+
dialect.blocks.set('value', function(params) {
9+
var value = params.value;
10+
11+
var result;
12+
if (_.isUndefined(value) || _.isNull(value)) {
13+
result = 'null';
14+
} else if (_.isBoolean(value)) {
15+
result = String(Number(value));
16+
} else if (_.isNumber(value)) {
17+
result = String(value);
18+
} else if (_.isString(value) || _.isDate(value)) {
19+
if (dialect.builder.options.separatedValues) {
20+
result = dialect.builder._pushValue(value);
21+
} else {
22+
if (_.isDate(value)) value = value.toISOString();
23+
result = '\'' + value + '\'';
24+
}
25+
} else {
26+
result = parentValueBlock(params);
27+
}
28+
29+
return result;
30+
});
31+
632
dialect.blocks.set('limit', function(params) {
7-
return (!isNaN(params.offset)) ? '' : 'top(' + dialect.builder._pushValue(params.limit) + ')';
33+
var result = '';
34+
if (_.isUndefined(params.offset)) {
35+
result = 'top(' + dialect.builder._pushValue(params.limit) + ')';
36+
}
37+
return result;
838
});
939

1040
dialect.blocks.set('offset', function(params) {
11-
var pre = (!params.sort) ? 'order by 1 ' : '';
41+
var prefix = (!params.sort) ? 'order by 1 ' : '';
42+
var offset = 'offset ' + dialect.builder._pushValue(params.offset) + ' ';
43+
var limit = '';
44+
var suffix = 'rows';
45+
1246
if (params.limit) {
13-
var str = pre + 'offset ' + dialect.builder._pushValue(params.offset);
14-
str += ' rows fetch next ' + dialect.builder._pushValue(params.limit) + ' rows only';
15-
return str;
16-
}else {
17-
return pre + 'OFFSET ' + dialect.builder._pushValue(params.offset) + ' rows';
47+
limit = 'rows fetch next ' + dialect.builder._pushValue(params.limit) + ' ';
48+
suffix = 'rows only';
1849
}
50+
51+
return prefix + offset + limit + suffix;
1952
});
2053

2154
dialect.blocks.set('returning', function(params) {
@@ -28,32 +61,30 @@ module.exports = function(dialect) {
2861

2962
dialect.blocks.set('insert:values', function(params) {
3063
var values = params.values;
64+
var fields = params.fields;
65+
var returning = params.returning;
3166

3267
if (!_.isArray(values)) values = [values];
3368

34-
var fields = params.fields || _(values)
35-
.chain()
36-
.map(function(row) {
37-
return _(row).keys();
38-
})
39-
.flatten()
40-
.uniq()
41-
.value();
69+
fields = fields || _(values)
70+
.chain()
71+
.map(function(row) {
72+
return _(row).keys();
73+
})
74+
.flatten()
75+
.uniq()
76+
.value();
77+
78+
values = _(values).map(function(row) {
79+
return _(fields).map(function(field) {
80+
return dialect.buildBlock('value', {value: row[field]});
81+
});
82+
});
4283

4384
return dialect.buildTemplate('insertValues', {
85+
values: values,
4486
fields: fields,
45-
returning: params.returning || undefined,
46-
values: _(values).map(function(row) {
47-
return _(fields).map(function(field) {
48-
return dialect.buildBlock('value', {value: row[field]});
49-
});
50-
})
87+
returning: returning
5188
});
5289
});
53-
54-
dialect.blocks.add('insertValues:values', function(params) {
55-
return _(params.values).map(function(row) {
56-
return '(' + row.join(', ') + ')';
57-
}).join(', ');
58-
});
5990
};

‎lib/dialects/mssql/index.js

+4-31
Original file line numberDiff line numberDiff line change
@@ -7,45 +7,18 @@ var templatesInit = require('./templates');
77
var blocksInit = require('./blocks');
88

99
var Dialect = module.exports = function(builder) {
10-
11-
builder._pushValue = function(value) {
12-
if (_.isUndefined(value) || _.isNull(value)) {
13-
return 'null';
14-
} else if (_.isBoolean(value)) {
15-
return String(Number(value));
16-
} else if (_.isNumber(value)) {
17-
return String(value);
18-
} else if (_.isString(value) || _.isDate(value)) {
19-
if (this.options.separatedValues) {
20-
var placeholder = this._getPlaceholder();
21-
22-
if (this.options.namedValues) {
23-
this._values[placeholder] = value;
24-
} else {
25-
this._values.push(value);
26-
}
27-
28-
return this._wrapPlaceholder(placeholder);
29-
} else {
30-
if (_.isDate(value)) value = value.toISOString();
31-
32-
return '\'' + value + '\'';
33-
}
34-
} else {
35-
throw new Error('Wrong value type "' + (typeof value) + '"');
36-
}
37-
};
38-
3910
BaseDialect.call(this, builder);
4011

4112
// init templates
4213
templatesInit(this);
4314

4415
// init blocks
4516
blocksInit(this);
46-
4717
};
4818

4919
util.inherits(Dialect, BaseDialect);
5020

51-
Dialect.prototype.config = _({}).extend(BaseDialect.prototype.config, {});
21+
Dialect.prototype.config = _({}).extend(BaseDialect.prototype.config, {
22+
identifierPrefix: '[',
23+
identifierSuffix: ']'
24+
});

‎lib/dialects/mssql/templates.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ module.exports = function(dialect) {
4646
}
4747
});
4848

49-
dialect.templates.add('insert', {
49+
dialect.templates.set('insert', {
5050
pattern: '{with} {withRecursive} insert {or} into {table} {values} ' +
51-
'{condition}',
51+
'{condition}',
5252
validate: function(type, params) {
5353
templateChecks.onlyOneOfProps(type, params, ['with', 'withRecursive']);
5454
templateChecks.propType(type, params, 'with', 'object');
@@ -68,7 +68,7 @@ module.exports = function(dialect) {
6868
}
6969
});
7070

71-
dialect.templates.add('insertValues', {
71+
dialect.templates.set('insertValues', {
7272
pattern: '({fields}) {returning} values {values}',
7373
validate: function(type, params) {
7474
templateChecks.requiredProp('values', params, 'fields');
@@ -83,7 +83,7 @@ module.exports = function(dialect) {
8383
}
8484
});
8585

86-
dialect.templates.add('update', {
86+
dialect.templates.set('update', {
8787
pattern: '{with} {withRecursive} update {or} {table} {alias} {modifier} {returning} ' +
8888
'{condition} ',
8989
validate: function(type, params) {
@@ -109,7 +109,7 @@ module.exports = function(dialect) {
109109
}
110110
});
111111

112-
dialect.templates.add('remove', {
112+
dialect.templates.set('remove', {
113113
pattern: '{with} {withRecursive} delete from {table} {returning} {alias} {condition} ',
114114
validate: function(type, params) {
115115
templateChecks.onlyOneOfProps(type, params, ['with', 'withRecursive']);

‎tests/6_dialects/1_mssql.js

+12-8
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ describe('MSSQL dialect', function() {
1717
'name': {$eq: 'test'}
1818
}
1919
});
20-
expect(result.query).to.be.equal('select top(1) "user" from "test" where "name" = $1;');
20+
expect(result.query).to.be.equal('select top(1) [user] from [test] where [name] = $1;');
2121
});
2222

2323
it('should be ok with `limit` and `offset` properties', function() {
@@ -30,7 +30,7 @@ describe('MSSQL dialect', function() {
3030
'name': {$eq: 'test'}
3131
}
3232
});
33-
expect(result.query).to.be.equal('select "user" from "test" where "name" = $1 order by 1' +
33+
expect(result.query).to.be.equal('select [user] from [test] where [name] = $1 order by 1' +
3434
' offset 2 rows fetch next 4 rows only;');
3535
});
3636
});
@@ -39,25 +39,29 @@ describe('MSSQL dialect', function() {
3939
var result = jsonSql.build({
4040
type: 'remove',
4141
table: 'test',
42-
returning: ['DELETED.*'],
42+
returning: [
43+
{table: 'deleted', name: '*'}
44+
],
4345
condition: {
4446
Description: {$eq: 'test'}
4547
}
4648
});
47-
expect(result.query).to.be.equal('delete from "test" output "DELETED".* where ' +
48-
'"Description" = $1;');
49+
expect(result.query).to.be.equal('delete from [test] output [deleted].* where ' +
50+
'[Description] = $1;');
4951
});
5052
it('should be ok with `insert` type', function() {
5153
var result = jsonSql.build({
5254
type: 'insert',
5355
table: 'test',
54-
returning: ['INSERTED.*'],
56+
returning: [
57+
{table: 'inserted', name: '*'}
58+
],
5559
values: {
5660
Description: 'test',
5761
}
5862
});
59-
expect(result.query).to.be.equal('insert into "test" ("Description") output ' +
60-
'"INSERTED".* values ($1);');
63+
expect(result.query).to.be.equal('insert into [test] ([Description]) output ' +
64+
'[inserted].* values ($1);');
6165
});
6266
});
6367
});

0 commit comments

Comments
 (0)