Repository navigation
Conversation
The NAN/V8 code only built up to Node 9 (upstream reached Node 12) and broke on every Node major. Rewrite it on node-addon-api so one build keeps working on Node 18 and later; the JavaScript API, results and error messages stay the same (checked against upstream 1.0.27 on Node 12). - Async calls run as AsyncWorkers that keep the connection and statement objects alive until their callback; a connection that is garbage collected detaches its statements instead of leaving dangling pointers - Exceptions thrown by callbacks are uncaught exceptions, as before - Report errors that happen while fetching rows (upstream fix) - Return rows whose columns are all NULL, which were dropped - Calling drop() twice and createConnection() with a number no longer crash the process - A prepared statement resets its batch size after a wide insert - Free bind parameter buffers that leaked, track the DBCAPI version once per process - Install with plain node-gyp rebuild; drop the Node 0.10 sources - Add node:test integration tests against a running server Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DBCAPI binds a NULL value of a multi-row bind as DT_NOTYPE with length 0 and other values with their own type, and the server rejects such a SQLDA (-700 "SQLDA fields inconsistent for a multi-row SQLDA"). No bind layout avoids it, so when a column mixes NULL and other values the rows are now executed one at a time and the affected rows are summed. A savepoint keeps the statement atomic: if a row fails, the rows before it are rolled back, as a failing multi-row execution inserts nothing. Also drop the batch size cached on prepared statements: sqlany_reset() already sets the batch size back to 1, so executing a prepared statement again with as many rows as before skipped sqlany_set_batch_size() and silently inserted only the first row. It now calls it for every multi-row execution, as upstream did, and fails with -182 instead: DBCAPI only executes several rows again if there are more than before (README). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DBCAPI's sqlany_reset() leaves room for one row in the SQLDA, and sqlany_set_batch_size() only restores it when it has to grow the SQLDA, so a prepared statement could only execute several rows again with more rows than any earlier execution (error -182 otherwise). Executions with several rows now use a second DBCAPI handle of the statement, prepared on first use and never reset; single-row executions keep resetting the main handle, which they need. Rows that leave parameters unbound still go to the main handle, as the second one would keep the bindings to an earlier execution's freed buffers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a call passed fewer values than the statement has placeholders, or none at all, the missing parameters were left unbound. The server takes them as NULL, but then caches a plan that keeps them NULL: later executions on the connection lost the values bound to those parameters, also for a new prepared statement, conn.exec() and other INSERT statements into the same table. A partial multi-row execution followed by a full one of the same INSERT crashed dbsrv17 17.0.0.1063. Setting max_plans_cached = 0 avoids both, and explicit NULL binds don't cause them. Missing values are now bound as NULL, so no input parameter stays unbound and multi-row executions of prepared statements always use the second handle. A procedure parameter that returns output stays unbound without a value, as DBCAPI would write the output into the NULL's missing buffer. A statement with placeholders executed without values now runs with NULLs instead of failing with -188 for some statements. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After sqlany_reset(), DBCAPI describes the IN parameters of a CALL that also has OUT parameters as OUT ones. Binding a value to an OUT parameter then crashed the process, and calls that left it out passed NULL for the IN parameters. No handle is reset any more: every multi-row execution of a prepared statement goes to the wide handle, so the main one only runs single rows. OUT parameters are never bound, ignoring their values, and INOUT ones are bound as input only, so DBCAPI never writes output into the driver's buffers (or an earlier execution's freed ones). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SQL Anywhere 16 fails INSERT INTO #calls (a, s) VALUES (a, ...) inside a procedure whose parameter is also called a with -143 "Column 'a' not found", even for a CALL without bind parameters. With distinct names the test passes unchanged against a 16 server (SQLA 17.0.4 client, Node 22 and 24) as well as against 17. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Thanks a lot for the PR! This package is effectively deprecated though, and having customers directly compile things through node-gyp is undesirable... Did you know that we have https://www.npmjs.com/package/@sap/iq-client now? It can be used with SQL Anywhere. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The NAN/V8 addon broke with every Node.js major release (#35, #42, #44, #43). This PR rewrites the native layer on Node-API with
node-addon-api, as #43 suggested, so that one build keeps working on Node.js 18 and later. The JavaScript API, result shapes and error messages stay the same.Changes
Napi::AsyncWorkers. They keep the connection and statement objects alive until their callback has run. An exception thrown by a callback is still reported as an uncaught exception.node-gyp rebuild.build.js,nanand the Node 0.10 sources are removed.enginesis>=18, and the version is 2.0.0.node:test(test/sqlanywhere.test.js) and run against a server given bySQLANY_TEST_CONN.Fixes
drop()twice, orcreateConnection()with a number, crashed the process.sqlany_reset(), DBCAPI describes the IN parameters of such a CALL as OUT ones. Statements are no longer reset, OUT parameters are not bound, and INOUT parameters are bound as input only.Behaviour changes
?placeholders are passed, the missing ones are bound as NULL. Before, some statements failed with -188. Others left the parameters unbound, and the server then cached a plan that kept them NULL in later executions. On 17.0.0.1063 that sequence could crash the server.Testing
SQL Anywhere 17 for macOS is x86_64 only, so on Apple silicon the tests need an x64 Node.js build.
🤖 Generated with Claude Code