Problem with passing null values into Binary, VarBinary and Image about node-sqlserver-v8 HOT 10 CLOSED

to confirm

function nullBinary() {
var msnodesql = require('node-sqlserver-v8');
msnodesql.open(connStr, function(err, conn) {
if (err) return console.error(err);

    conn.query("declare @bin binary(4) = ?; select @bin as bin", [new Buffer(0)], function(err, res) {
        console.error(err);
        console.error(res);
    });
});

}

i.e. you are saying passing in new Buffer(0) does not work? when i run the above, i do not see this error. Running with null, agreed it does fail and this is because actually the driver tries to decode type from params rather than column meta data which would be more reliable, but that is a pretty big change to make and is consistent with previous behavior.

can you confirm what you are expecting to see when new Buffer(0) is sent in?

i see

Debugger listening on port 16763
null
{ bin: <Buffer 00 00 00 00> }

from node-sqlserver-v8.

I made some tests and realised that the problem is that new Buffer(0) is a valid value and should not be converted to null. The workaround I used to use was a bug and should be no longer used.

var msnodesql = require('msnodesqlv8');
msnodesql.open(connectionString, function(err, conn) {
    if (err) return console.error(err);

    conn.query("declare @bin varbinary(max) = ?; insert into varbintest(bin) values (@bin); select @bin as bin", [new Buffer(0)], function(err, res) {
        console.log(res);
    });
});

This code inserts one row to table varbintest with value 0x and returns it back as a result.

[ { bin: <Buffer > } ]

I have no idea how to resolve this without knowing the parameter's type.

from node-sqlserver-v8.

can you see if the following is any help. I only support varBinary at the moment through this way of binding, but it can be extended. The idea is to tell the c++ driver explicitly what the type is rather than it trying to guess. In this way we can pass a null of type varbinary, I added the test to the params suite.

open(function(conn) {
var tm = conn.tableMgr();
conn.query("declare @bin binary(4) = ?; select @bin as bin", [tm.sqlTypes.asVarBinary(null)], function (err, res) {
console.log(res[0]['bin'] == null);
console.error(err);
console.error(res);
});
});

from node-sqlserver-v8.

I absolutely preffer this way of passing arguments. Automatic type guess is a nice feature until you need to explitily set something.

The code looks unnecessarily verbose to me. Creating a table manager to cast types seems like overkill to me. Some ideas:

var msnodesql = require('msnodesqlv8');
msnodesql.open(connectionString, function(conn) {
    conn.query("select @bin", [msnodesql.VarBinary(null)], function(err, res) {});
    // or
    conn.query("select @bin", [msnodesql.cast(null, "VarBinary")], function(err, res) {});
})

from node-sqlserver-v8.

agreed, your API is more elegant. I like both methods, I would probably say the first would be my preference. I will take a look and get back to you when I have something.

from node-sqlserver-v8.

OK, I added some new methods.

let me know if you think the names etc should be changed. I have added a new test suite userbind.js

sql.Double(d)
sql.BigInt(i)
sql.VarBinary(null)
sql.Integer(i)
sql.WVarChar(str)
sql.SSTimeStampOffset(utcDate)

once things look OK I will update the documentation.

I removed the first attempt as previously discussed.

from node-sqlserver-v8.

I would prefer naming according to type. So it would be nice to have all those functions available.

sql.Bit(value)
sql.BigInt(value)
sql.Decimal(value, [precision], [scale]) -- optional precision and scale definition
sql.Float(value)
sql.Int(value)
sql.Money(value)
sql.Numeric(value, [precision], [scale]) -- optional precision and scale definition
sql.SmallInt(value)
sql.SmallMoney(value)
sql.Real(value)
sql.TinyInt(value)

sql.Char(value, [length]) -- optional length definition
sql.NChar(value, [length]) -- optional length definition
sql.Text(value)
sql.NText(value)
sql.VarChar(value, [length]) -- optional length definition
sql.NVarChar(value, [length]) -- optional length definition
sql.Xml(value)
sql.UniqueIdentifier(value)

sql.Time(value, [scale]) -- optional scale definition
sql.Date(value)
sql.DateTime(value)
sql.DateTime2(value, [scale]) -- optional scale definition
sql.DateTimeOffset(value, [scale]) -- optional scale definition
sql.SmallDateTime(value)

sql.Binary(value)
sql.VarBinary(value, [length]) -- optional length definition
sql.Image(value)

sql.UDT(value)
sql.Geography(value)
sql.Geometry(value)

sql.Variant(value)

from node-sqlserver-v8.

OK, I am still working on this, it will probably be available within the next week. I will add a base set of unit tests but we will only know the limitations through use. Ill keep you posted with progress.

from node-sqlserver-v8.

I did have a go at adding quite a few of these into the API with some tests. If you still need them, take a look else they will be there for others. The null into varbinary is available which is what kicked off this discussion.

from node-sqlserver-v8.

I will assume this is implemented ok now. For further problems with user binding please raise a specific test covering that type and I will take a look. The specific user binding mentioned is now available. Therefore I'll close this issue.

from node-sqlserver-v8.