SET defaults

[kppullin]

Updated the initial sql call to set default settings flags to match those generated by ADO.NET/SQL Server Management Studio, as validated by SQL Server Profiler.

These are considered "sane" & "least suprise" defaults (for example, linked server queries fail without a couple settings here, etc).

The same effect may be forced by the ODBC_ON flag in login7-payload.coffee, however I'm unaware if this can cause other unintended side effects (again, settings can be verified in SQL Server Profiler).

This also addresses issue #59

[pekim]

Thanks. This looks pretty reasonable. I should be able to take a better look at the weekend, including running the integration tests.

We need to think about whether it's a good idea setting the transaction isolation level. This is already defaulted in the connection options (http://pekim.github.com/tedious/api-connection.html#function_newConnection), and I'm inclined to think that that's the way it should stay. Although perhaps the default there is wrong, and it should default to READ_COMMITTED?

[kppullin]

Thanks for the response & feedback! If you've additional suggestions on adding tests or other changed I'd be happy to help : )

Regarding the isolation level, IMHO it should be set to READ_COMITTED instead of READ_UNCOMITTED.

Reasons being: 1) this is the default as shipped by Microsoft and adheres to the principle of least surprise (at least for those coming from a SQL Server/.NET ecosystem), and 2) on a personal level, I like to avoid dirty reads where possible, and only use READ_UNCOMITTED where I know and have accepted the risks.

Additionally, from my brief look at the code, the 'config.isolationLevel' setting is only applied for transactions. Items outside of the transaction use the default isolation level or SQL Server, which is READ_COMMITTED.

So while this would be a breaking change with respect to the documentation, the current implementation only adheres to the documented behavior when using explicit transactions.

For reference, here are the defaults applied in the 'Audit Login' profiler event when connecting with tedious outside of a transaction:

set quoted_identifier off
set arithabort off
set numeric_roundabort off
set ansi_warnings off
set ansi_padding off
set ansi_nulls off
set concat_null_yields_null off
set cursor_close_on_commit off
set implicit_transactions off
set language us_english
set dateformat mdy
set datefirst 7
set transaction isolation level read committed

[jonbuffington]

@pekim Could you publish a 0.1.4 release that includes this change? Running into an issue that would be resolved by this PR.

[pekim]

Sure. I'll try to do it within the next week. Probably on Saturday.

Mike

On 10 April 2013 15:24, Jon Buffington notifications@github.com wrote:

@pekim https://github.com/pekim Could you publish a 0.1.4 release that
includes this change? Running into an issue that would be resolved by this
PR.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/83#issuecomment-16177178
.

[jonbuffington]

@pekim Thanks!