wankdanker / node-odbc

ODBC bindings for node
MIT License
174 stars 79 forks source link

Improvements to GetColumnValue #9

Open lee-houghton opened 10 years ago

lee-houghton commented 10 years ago

I've had a chance to merge the changes from 0.6.2 to 0.6.11 into my fork. I'm currently testing it myself.

There are some compatibility changes:

I'd like to implement sending node Buffer objects for SQL_BINARY parameters, but I haven't yet had the time.

It's not been thoroughly tested, especially on platforms other than Windows/MSSQL, so there may certainly be issues there. I don't have any other platforms set up, so I can't currently test it on those without a lot of work.

wankdanker commented 10 years ago

Awesome! Thank you for re-working this onto v0.6.11. When I get a chance to understand this in more detail and test on Linux with FreeTDS, Sqlite, MySQL and Postgres I'll probably publish it as v0.7.0 because of the compatibility changes you mentioned.

wankdanker commented 10 years ago

Is your Windows environment 32 or 64 bit?

lee-houghton commented 10 years ago

I'm building on a 64-bit machine, but my Node.js version is 32-bit at the moment. It's possible I've introduced some changes which break 64-bit compatibility - I apologise if that's the case :(

wankdanker commented 10 years ago

No worries. I just could not build because the call to SQLGetData according to spec uses SQLLEN for the BufferLength argument and in several instances in your code the variable being passed is of SQLINTEGER type. On my system there is no conversion available from SQLINTEGER to SQLLEN. So, I had to change a bunch of things over to SQLLEN. No problem, was just wondering how that might have worked for you.

At any rate, I got it to build but with lots of failing tests. :( Possibly because I didn't really pay close attention to what I was doing as explained above. ;)

lee-houghton commented 10 years ago

Hopefully I should have time tomorrow to fix the break I introduced for 64-bit Linux builds and do some more thorough testing with the msodbcsql11 driver.

wankdanker commented 10 years ago

OK. Sounds good. I've been slammed with non-related work, so I haven't had much time to address any of the pull requests. :(

lee-houghton commented 10 years ago

OK, I'll need to test this further, but I that seems to work. I had to also change the buffer-reading code for integers to expect sizeof(SQLINTEGER) bytes in the buffer instead of sizeof(long).

lee-houghton commented 10 years ago

Just added another fix - I noticed that date values before 1970 weren't being handled correctly in Windows (it was always returning 1969-12-31T23:59:59 for those dates) because MSVC's _mkgmtime32 doesn't handle them.