Conversation
Mssql is now using builder._pushValue for limit/offset Replaced whitspaces with tabs
|
Hi, @iteufel. |
Replaced spaces with tabs Reverted package.json
|
Hi @okv,
|
|
@iteufel, awesome. |
|
|
||
| var Dialect = module.exports = function(builder) { | ||
|
|
||
| builder._pushValue = function(value) { |
There was a problem hiding this comment.
You shouldn't override builder._pushValue, you should override value block and do your specific stuff before parent value block call. See example in postgresql dialect: https://github.com/2do2go/json-sql/blob/master/lib/dialects/postgresql/blocks.js#L6
|
|
||
| module.exports = function(dialect) { | ||
| dialect.blocks.set('limit', function(params) { | ||
| return (!isNaN(params.offset)) ? '' : 'top(' + dialect.builder._pushValue(params.limit) + ')'; |
There was a problem hiding this comment.
Maybe _.isUndefined check is better here?
return _.isUndefined(params.offset) ? 'top(' + dialect.builder._pushValue(params.limit) + ')' : '';There was a problem hiding this comment.
And why you check offset value, but return limit value?
There was a problem hiding this comment.
MSSQL has no LIMIT/OFFSET Statement like mysql.
There are 2 diffrent ways:
SELECT TOP X Fields... is the same as LIMIT.
But you cannot use OFFSET together with TOP. IN MSSQL 2012 and higher a new statement was added.
SELECT fields from ... ORDER BY A OFFSET Y ROWS FETCH NEXT X ROWS ONLY
That is why he checks for the params.offset. If it is used the top() statement will be removed and the offset/fetch statement will include the limit.
| var str = pre + 'offset ' + dialect.builder._pushValue(params.offset); | ||
| str += ' rows fetch next ' + dialect.builder._pushValue(params.limit) + ' rows only'; | ||
| return str; | ||
| }else { |
There was a problem hiding this comment.
whitespace is missed here
|
|
||
| return dialect.buildTemplate('insertValues', { | ||
| fields: fields, | ||
| returning: params.returning || undefined, |
There was a problem hiding this comment.
Why returning is needed here?
| }); | ||
| }); | ||
|
|
||
| dialect.blocks.add('insertValues:values', function(params) { |
There was a problem hiding this comment.
It is seems that this block is not needed to override
| str += ' rows fetch next ' + dialect.builder._pushValue(params.limit) + ' rows only'; | ||
| return str; | ||
| }else { | ||
| return pre + 'OFFSET ' + dialect.builder._pushValue(params.offset) + ' rows'; |
There was a problem hiding this comment.
all words should be in lower case, you use offset word above but here OFFSET
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.
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.
4 participants
Added support for MSSQL.
Following should now work: