Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to set the default isolation level for the connection #156

Merged
merged 3 commits into from
Jun 24, 2014
Merged

Add option to set the default isolation level for the connection #156

merged 3 commits into from
Jun 24, 2014

Conversation

rossipedia
Copy link
Contributor

We have a requirement that all queries use READ UNCOMMITTED by default. This PR adds an option, connectionIsolationLevel, that allows setting the isolation level in the initial sql batch that gets sent when a connection is opened (defaulting to READ COMMITTED so as not to change existing behavior).

Also, I noticed that using the single quoted block string was causing the newline to be trimmed, resulting in the wrong sql being sent

set textsize 2147483647set quoted_identifier on

instead of

set textsize 2147483647
set quoted_identifier on

Changing the batch to use a single double quoted block string and using coffee-script's interpolation fixes the issue.

Usage would be:

var tds = require('tedious');

var conn = new tds.Connection({
  ...
  options: {
    connectionIsolationLevel: tds.ISOLATION_LEVEL.READ_UNCOMMITTED
  }
});

@rossipedia
Copy link
Contributor Author

Alternatively, the existing isolationLevel property could be re-used, although that would make it so all normal queries would have the same isolation level as explicit transactions.

@bretcope
Copy link
Member

Another alternative is that it could be simply:

@config.options.connectionIsolationLevel ||= @config.options.isolationLevel

And the documentation could read something like:

Each new connection will be opened with this isolation level by default. Defaults to the value of options.isolationLevel.

The only issue with this is that it could change behavior if someone was intentionally expecting a different isolation level for general queries vs. transactions. It seems sensible to expect the same default for both, but either way will address our needs.

Obviously the docs for github.io should definitely be updated when this gets accepted.

@bretcope bretcope mentioned this pull request Jun 23, 2014
patriksimek added a commit that referenced this pull request Jun 24, 2014
Add option to set the default isolation level for the connection
@patriksimek patriksimek merged commit 48237e5 into tediousjs:master Jun 24, 2014
@rossipedia rossipedia deleted the default-isolation-level branch June 24, 2014 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants