-
Notifications
You must be signed in to change notification settings - Fork 443
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
VarChar encoding problem. #723
Comments
maybe relate to #113 , but it should use |
@heroboy varchar stores 1 byte (ascii) characters, there's an equivalent function in nvarchar datatype object to handle the Unicode characters tedious/src/data-types/nvarchar.js Lines 55 to 62 in 85d96fe
If you have faced an issue would you please send a repo? |
hi @Hadis-Fard, db.acquire((err, conn) =>
{
if (err) return;
var request = new tedious.Request("select '中文',@v1,@v2,@v3; set @v4 = '中文';", function (err)
{
if (err)
{
console.log(err);
return;
}
conn.release();
});
const iconv = require('iconv-lite');
request.on('row', cols =>
console.log(cols.map(x => x.value))
);
request.on('returnValue', (name, value) =>
{
console.log(name, value);
})
console.log(iconv.encode('中文', 'cp936'));
request.addParameter('v1', tedious.TYPES.VarChar, '中文')
request.addParameter('v2', tedious.TYPES.NVarChar, '中文')
request.addParameter('v3', tedious.TYPES.VarChar, iconv.encode('中文', 'cp936'),{length:128})
request.addOutputParameter('v4', tedious.TYPES.VarChar, null);
conn.execSql(request);
}) The output is
I tried use buffer as the parameter value,but it didn't work. Becuase in
And the output would be
The |
And I don't understand what you mean about |
as it is mentioned in MSDN varchar/char is non-Unicode string data, and nvarchar/nchar is unicode string data. so when you have a Unicode string data you should use nvarchar. Also we want to validate the char parameters to be string or if it is possible we convert it to string |
If a table column type is varchar, it can correctly store '中文'( Do you think it is an unicode string data that can't be stored?)。For example, the sql is Another example, I see in |
What I do |
So please at least make the parameter value can be a buffer. It's simple to fix. |
@heroboy 👋 Sorry you're running into this issue with You're right, Unfortunately, query parameters do not support specifying an encoding option, but it should be possible to add this. I'm against allowing I think the correct way to support this is to store the SQL Collation that is send to the client via the @heroboy @Hadis-Fard What do you think? |
Actually, I just saw that we have an internal |
@arthurschreiber When writing code, we should always keep in mind that when converting between string and binary buffer, we should always specify an encoding. Using If you can read SqlServer's encoding setting. https://stackoverflow.com/questions/5182164/sql-server-default-character-encoding It is best and correct. If just adding an encoding in tedious option(or Thanks. |
Using collation received in |
Both ADO and JDBC driver are sending string as NVarchar by default. Here is the TSQL sent by the drivers (captured via SQL Profiler) -- JDBC
exec sp_executesql N'INSERT INTO c936_table (c1) VALUES(@P0)',N'@P0 nvarchar(4000)',N' 中文'
--ADO
exec sp_executesql N'INSERT INTO [dbo].[c936_table] ([c1]) VALUES (@c1);',N'@c1 nvarchar(11)',@c1=N'中文'
--tedious
exec sp_executesql @statement=N'insert into c936_table values (@c1);',@params=N'@c1 varchar(2)',@c1='-‡' I was sending value '中文' as string/varchar in all 3 cases, and this was the result. Passing string as NVarchar by default seems to be a better alternative than using database collation on all of the char/varchar columns. Using NVarchar too comes at a cost, but considering that collation can be set at any level (expression, column, database or Server) its better to have a solution to handle them all or none at all. |
@v-suhame |
I'm reopening because this is still an issue - see the discussion in #1294. @heroboy You posted some code to reproduce this behaviour over in #723 (comment). Do I need to tweak any additional server or database settings (e.g. change the server collation) to reproduce this correctly? 🙇♂️ |
@arthurschreiber request.addParameter('v3', tedious.TYPES.VarChar, iconv.encode('中文', 'cp936'),{length:128}) You should set sql server to match the |
see this:
tedious/src/data-types/varchar.js
Lines 55 to 68 in c22ed18
You use the ascii encoding, is it incorrect?
I see you consider the encoding in
value-parser.js
.So I find that the input parameter encoding is not correct, the select result is correct.
The text was updated successfully, but these errors were encountered: