Skip to content

Port to Node-API: support Node.js 18 and later - #52

Open
alevinru wants to merge 6 commits into
sqlanywhere:masterfrom
Sistemium:node-api
Open

alevinru wants to merge 6 commits into
sqlanywhere:masterfrom
Sistemium:node-api

Conversation

@alevinru

Copy link
Copy Markdown

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

  • Async calls run as 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.
  • The module is installed with plain node-gyp rebuild. build.js, nan and the Node 0.10 sources are removed. engines is >=18, and the version is 2.0.0.
  • Integration tests use node:test (test/sqlanywhere.test.js) and run against a server given by SQLANY_TEST_CONN.

Fixes

  • Rows whose columns were all NULL were missing from results.
  • Calling drop() twice, or createConnection() with a number, crashed the process.
  • A connection that was garbage collected before its statements left them with dangling pointers.
  • Bind buffers leaked. The DBCAPI version is now tracked once per process instead of per connection, where it was uninitialized after the first connection.
  • Wide (array-of-arrays) statements with a column that mixes NULL and other values failed with -700. Such batches now run one row at a time inside a savepoint. A prepared statement can run batches of any size, one after another.
  • Binding a value to an OUT procedure parameter crashed the process. After 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

  • If fewer values than ? 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.
  • Wide statements that mix NULL and other values now succeed instead of failing with -700.

Testing

  • Linux x86_64 (Ubuntu 24.04), Node.js 22.22 and 24.22, SQL Anywhere 17.0.4 client against a 16.0.0.2366 server: all 23 tests pass. With a 16 client, wide statements report -2009 as before and everything else passes.
  • macOS x86_64, Node.js 24, SQL Anywhere 17.0.0.1063 client and server: all 23 tests pass, also under AddressSanitizer.
  • A script of about 90 calls gave the same results and error messages on 1.0.27 with Node 12 as on this branch with Node 24, except for the behaviour changes above.

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

alevinru and others added 6 commits October 16, 2026 11:32
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>
@jeffalbion

Copy link
Copy Markdown
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants