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

Need to add prefix 'N' before Unicode string when work with MSSQL #3752

Closed
colinyuran opened this Issue May 19, 2015 · 11 comments

Comments

7 participants
@colinyuran

colinyuran commented May 19, 2015

I am new to Sequelize and NodeJS, need to use it work with SQL Server 2014, and I also need to deal with Unicode string in my table columns.

In my model definition, I define a Name field with type DataTypes.STRING(255), the final real database column type is NVARCHAR(255). I can insert data with Unicode string as below sql statement:

insert into [Test] values (N'我們')

Database can store the Chinese string correctly.

But when I use sequelize API, such as Model.create method, I will get question mark ( ???? ) in name column. After I check the NodeJS log, I find out the SQL statement generated by sequelize is as below:

INSERT INTO [Test] ([Name]) OUTPUT INSERTED.* VALUES ('我們');

There is no prefix 'N' before the Chinese string, this is why it can't store correctly in Database.

I debug by myself but don't find a graceful way to change this behavior, is there any configuration not mentioned in the document?

I also submit a question in stackoverflow with more details (include my codes and my debug findings, also with an ugly temporary fix). Here is the link:
http://stackoverflow.com/questions/30303841/how-to-make-sequelize-add-n-before-unicode-string-in-its-sql-statement

@janmeier

This comment has been minimized.

Member

janmeier commented May 19, 2015

Are there any reverse implications to adding the N - I mean, will there ever be cases where you don't want to have the N prefixed?

@mbroadst

This comment has been minimized.

Contributor

mbroadst commented May 19, 2015

@janmeier iirc no. I believe that putting an N before a string literal simply converts it to an NVARCHAR unicode representation. The tradeoff here is that you might not want the extra space in the on-disk representation, but we're already assuming that STRING == NVARCHAR, so if the tests pass with everything being switched to prefix with N then I'd say you're probably good to go. Maybe someone knows more than I do though 😄

@janmeier

This comment has been minimized.

Member

janmeier commented May 20, 2015

Yep, I'd say that's the way to go then. PR welcome @colinyuran :)

@colinyuran

This comment has been minimized.

colinyuran commented May 23, 2015

In my case, we need to store strings from many different languages in Database, so we prefer to define all string like types (char, text and so on) as unicode, so I am really glad the sequelize already translate all string type to NVARCHAR. But Microsoft SQL Server is very unique, even you define a table column as NVARCHAR which means the column is unicode, you still need to add the prefix N before the string literal value when you try to insert or update that column, this behavior is caused by some historical reason and Microsoft SQL Server need to keep compatible with old versions. So even I use Microsoft SQL Server management GUI to insert a column defined as NVARCHAR without the prefix 'N' , I still get ??? in the record.

Actually, the fix I pasted in stack overflow is really very simple and not consider the whole story throughly, I don't think the fix's quality is good enough to send a pull request. Because it will add prefix 'N' to all values which is in string format, such as string value and date value (it is string in the sql statement eventually).

What I think a better fix should be a define option which can allow the application author to ask to add prefix 'N' for nvarchar column, I think this is better.

@janmeier

This comment has been minimized.

Member

janmeier commented May 24, 2015

@colinyuran escapeId is used to esacpe identifiers so that won't work no - as I understand it we should prefix the N on the value, right?

As you say, it would probably be better to add a global sequelize option ala supportUnicode to turn Ns on and off.

The best solution would probably be to override query-generator.escape for mssql, similar to how it is done for postgres https://github.com/sequelize/sequelize/blob/master/lib/dialects/postgres/query-generator.js#L876

@kant2002

This comment has been minimized.

kant2002 commented Sep 5, 2015

I end up patching SqlString.escape from sequelize/lib/sql-string myself.

var SqlString = require("sequelize/lib/sql-string");
var DataTypes = require('sequelize/lib/data-types');

var originalEscape = SqlString.escape;
SqlString.escape = function (val, stringifyObjects, timeZone, dialect, field) {
    if (typeof val === "string" && (!field || field.type instanceof DataTypes.STRING)) {
        return "N" + originalEscape.call(null, val, stringifyObjects, timeZone, dialect, field);
    } else {
        return originalEscape.call(null, val, stringifyObjects, timeZone, dialect, field);
    }
};

Maybe that's way too simple and I found some consequences to that method, but so far it allows me work with Unicode.

@kant2002

This comment has been minimized.

kant2002 commented Sep 5, 2015

If I put code like that in the MSSql specific dialect, would it works? I'm not familiar with internals of this library, so interested what other checks possible needed.

@nuttycom

This comment has been minimized.

nuttycom commented Oct 6, 2017

Prefixing all values with N can cause horrific performance issues - often, this will cause queries to result in full table scans rather than index lookups. Is there a way to avoid this?

@kshateesh

This comment has been minimized.

kshateesh commented Jan 16, 2018

@nuttycom did you find a way to avoid Prefixing values with N ??

@nuttycom

This comment has been minimized.

nuttycom commented Jan 16, 2018

@kshateesh In a manner of speaking... which is to say that I quit having anything to do with the JavaScript world and am now happily writing Haskell instead. But, sorry, I didn't find a solution to your problem.

@kshateesh

This comment has been minimized.

kshateesh commented Jan 17, 2018

@nuttycom oh! It's Okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment