-
Notifications
You must be signed in to change notification settings - Fork 439
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
Set ansi_null_dflt_on on by default #230
Conversation
I like the consistency with SSMS and the native driver. I would note that this is a minor breaking change though. At the very least, there needs to be a config option to restore previous behavior. I actually think that you're right that a way to customize the initial sql is the best long-term approach. |
@lee-houghton I also like this change very much. Would you be open to work on making the initial sql customizable? |
@arthurschreiber - definitely. I think it would be best to do it in such a way that you don't have to re-specify all the default options, then if the defaults change the user will still receive new values for anything they haven't already customized. I'd probably lean towards the simplest option of allowing appending to the initial SQL via |
There are two separate issues here which are getting conflated, and that's a bad thing. This PR should address the original issue by changing the initial sql, and adding a config option which allows you to switch it back to the old behavior. That's how we've done changes to the initial sql in the past, and is the most respectful of our users. The second issue, which should probably be a separate PR, is how do we want to enable people to make arbitrary changes to the initial sql (stuff we won't support via the config). I think the simplest way to do that is with an optional function, but with a slightly different interface than the one you're proposing. Here's what I would suggest: config.options.getInitialSql = function (defaultInitialSql) {
return defaultInitialSql + "SET SOME OPTION;\n";
}; That would provide the default initial sql as a baseline for you to work with, but allows you to do whatever you want, including replace it entirely. |
To revert to the previous default, set config.options.enableAnsiNullDefault to false. This emulates the defaults for ODBC and OLE SQL. Also add a test that the default settings allow adding a null value to a temporary table with unspecified nullability.
That's actually how I initially considered doing it. Makes sense. I've changed my |
I'm in favor of merging this. Documentation also needs to be updated. If no one else gets to it in the next few days, I'll take care of it. |
Set ansi_null_dflt_on on by default
Published as |
I recently switched my application to use
tedious
for connecting to SQL Server 2012, which mostly went smoothly. I did however notice that some stored procedures produced errors when ran through atedious
connection, when they worked when run via ODBC or SQL Server Management Studio.By playing with the Query Options in SSMS I was able to find that I could reproduce the problem by unticking
SET ANSI_NULL_DFLT_ON
. According to MSDN the OLE DB and ODBC drivers set this flag by default. Perhapstedious
should set it too?At the same time, perhaps a way of customizing what
getInitialSql
returns would be useful, but I thought that wasn't really for me to decide. For the moment, I have duck-punched it like this: