node-odbc icon indicating copy to clipboard operation
node-odbc copied to clipboard

Improvements to GetColumnValue

Open lee-houghton opened this issue 11 years ago • 8 comments

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:

  • SQL_TYPE_DATE columns are handled (I don't think they were before)
  • Removed strptime.cpp on Windows - it now uses mktime and _mkgmtime32
  • SQL_BINARY columns are returned as a node Buffer object, rather than a hex-encoded string
  • SQL_BIT columns are handled using the SQL_C_BIT data type rather than converting to SQL_C_CHAR
  • The ODBCResult fetch mode can now be FETCH_NONE, which specifies that the columns will be fetched manually
  • SQL_VARCHAR and SQL_BINARY columns can be partially requested using the maxBytes parameter.

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.

lee-houghton avatar Jul 24 '14 13:07 lee-houghton

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 avatar Jul 24 '14 14:07 wankdanker

Is your Windows environment 32 or 64 bit?

wankdanker avatar Jul 24 '14 15:07 wankdanker

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 :(

lee-houghton avatar Jul 24 '14 15:07 lee-houghton

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. ;)

wankdanker avatar Jul 24 '14 16:07 wankdanker

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.

lee-houghton avatar Sep 03 '14 20:09 lee-houghton

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. :(

wankdanker avatar Sep 03 '14 20:09 wankdanker

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 avatar Sep 05 '14 08:09 lee-houghton

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.

lee-houghton avatar Oct 14 '14 13:10 lee-houghton