N-API/SQLBindCol changes

[markdirish]

Completely reworked the package to use N-API, specifically the node-addon-api C++ wrapper. This is an engine-agnostic abstraction that is ABI stable, meaning it doesn't need to be recompiled when new node versions are used.

Also, retooled the way that the ODBC drivers interact with the database. Currently, node-odbc uses SQLGetData, which is slow. Now uses SQLBindCol and then calls SQLFetch to populate a buffer array, from which N-API data is created and returned to JavaScript. When testing with 100k rows, SQLGetData took 19 minutes, while binding with SQLBindCol and getting data with SQLFetch took 19 seconds.

More information can be found at the bottom of the README.md file, including work and testing that still needs to be done.

[wankdanker]

Wow. Awesome. Can't wait to test it out.

[markdirish]

Ok, All of the tests should now be working (only tested on SQLite. Also issue54 syntax seems to only be written from MSSQL). Just need to ensure that memory is freed correctly throughout (particularly the QueryData struct)

[wankdanker]

The following tests are failing for me:

Running test for [Sqlite3] : test-describe-column ... fail  
Running test for [Sqlite3] : test-describe-database ... fail  
Running test for [Sqlite3] : test-describe-table ... fail  
Running test for [Sqlite3] : test-issue-54 ... fail  
Running test for [Sqlite3] : test-prepareSync-multiple-execution ... fail  
Running test for [Sqlite3] : test-query-create-table ... fail  
Running test for [Sqlite3] : test-query-drop-table ... fail  
Running test for [Sqlite3] : test-query-insert ... fail  
Running test for [Sqlite3] : test-queryResultSync-getRowCount ... fail  
Running test for [Sqlite3] : test-transaction-commit-sync ... fail  
Running test for [Sqlite3] : test-transaction-commit ... fail

I have your branch checked out through 7c51442c81f4137e4c3bf7c4ebb031241a48f3e6.

Are these tests passing for you?

Also, there is a core dump file committed to the repo. :)

[wankdanker]

We might be able to modify test-issue-54.js to run only if the connection dialect is mssql, which is configured in test/config.testConnectionStrings.json.

[markdirish]

Those tests are all working for me, but that list looks like many of the tests I worked on fixing. Did you rerun node-gyp build to rebuild the source? I deleted the core file, git add . biting me again!

The pool connection close test seems to fail about 50% of the time with a timeout, might be depedent on how fast my system is.

I will look into modifying test 54.

[wankdanker]

Those tests are all working for me, but that list looks like many of the tests I worked on fixing. Did you rerun node-gyp build to rebuild the source?

Yep, I rebuilt the module. The describe tests are failing with:

Error in exec direct query
/home/dverweire/projects/node/node-odbc/lib/odbc.js:213
        result.fetchAll(function (err, data) {
               ^

TypeError: Cannot read property 'fetchAll' of null
    at fetchMore (/home/dverweire/projects/node/node-odbc/lib/odbc.js:213:16)
    at cbQuery (/home/dverweire/projects/node/node-odbc/lib/odbc.js:206:7)

This is because there is actually an error being returned in Database.prototype.query.

The cbQuery function is actually getting an initial error:

{ errors: [], error: '[node-odbc] Error in ODBCConnection::QueryAsyncWorker', message: '[node-odbc] An error occurred but no diagnostic information was available.' }

and result is undefined.

Haven't dug any deeper yet.

I deleted the core file, git add . biting me again!

Doh! Been there. :)

[markdirish]

I have pushed up some further changes, but the vast majority are just refactoring for readability. @wankdanker any luck determining what was causing the deep error in QueryAsyncWorker? If there is an error in that method, it will return null for the result, so not surprising it cant run fetchAll, just concerning that an error is happening at all.

[wankdanker]

@markdirish Unfortunately I have not had a chance to look any further. I will try to take a look again tomorrow.

[markdirish]

Polite pester, any progress made at figuring out why tests fail for you?

[markdirish]

Closing as more changes were made in a different branch. See merge request 50 for more details.