Integration branch for saturno: query cancellation + TINYINT fix#1
Draft
rendonvelez wants to merge 12 commits into
Draft
Integration branch for saturno: query cancellation + TINYINT fix#1rendonvelez wants to merge 12 commits into
rendonvelez wants to merge 12 commits into
Conversation
Closes IBM#434. Add the ability to cancel in-flight operations: - connection.cancel(): ODBCConnection keeps a registry of statement handles for in-flight operations (std::set<SQLHSTMT> guarded by a uv_mutex_t). QueryAsyncWorker and CallProcedureAsyncWorker register their handle right after allocating it and deregister it in their destructors. cancel() walks the registry and calls SQLCancel on each handle. - statement.cancel(): calls SQLCancel directly on the statement's handle, which lives for the whole lifetime of the Statement object. The cancelled operations return with SQLSTATE HY008 ("Operation canceled"), so their promises reject (or their callbacks receive an error) and callers can distinguish cancellation from other failures. SQLCancel is invoked synchronously on the main thread rather than through an AsyncWorker: cancellation is most needed precisely when the libuv thread pool is saturated by the operations being cancelled, so queueing the cancel behind them could delay it indefinitely. Calling SQLCancel from a different thread than the one running the statement is the documented multithreaded-cancel use of the function. StatementData.hstmt is now initialized to SQL_NULL_HANDLE so that cleanup paths that run before the handle is allocated read a defined value. Signed-off-by: Gustavo Rendón Vélez <22383219+rendonvelez@users.noreply.github.com>
SQL_TINYINT columns and procedure parameters were bound as SQL_C_UTINYINT. When a driver returns a signed TINYINT with a negative value (e.g. MySQL, Impala, Hive), the conversion to an unsigned C type fails with SQLSTATE 22003 (numeric value out of range), so any SELECT over such a column errors out. Bind TINYINT to SQL_C_SHORT/SQL_C_SSHORT instead, the same path already used for SQL_SMALLINT. SQLSMALLINT covers the full range of both signed (-128..127) and unsigned (0..255) TINYINT, so drivers that expose TINYINT as unsigned (e.g. SQL Server) keep returning values above 127 correctly. This also fixes TINYINT output parameters in callProcedure, which had no SQL_C_UTINYINT case in the result conversion and fell through to the SQL_C_CHAR default. Signed-off-by: Gustavo Rendón Vélez <22383219+rendonvelez@users.noreply.github.com>
Signed-off-by: Gustavo Rendón Vélez <22383219+rendonvelez@users.noreply.github.com>
The data pointer is always valid: the only constructors of QueryAsyncWorker and CallProcedureAsyncWorker receive it as a required argument, so checking it for NULL before unregistering while dereferencing it unconditionally right after was inconsistent. Addresses review feedback. Signed-off-by: Gustavo Rendón Vélez <22383219+rendonvelez@users.noreply.github.com>
…comments For SQL_PARAM_INPUT_OUTPUT parameters, the TINYINT case overrode BufferLength to sizeof(SQLCHAR) without changing the value type that ODBC::StoreBindValues had already chosen (SQL_C_SBIGINT/SQL_C_DOUBLE for JS numbers). Route it through the same path as SQL_SMALLINT, which handles the numeric binding and the character-buffer resize. Also remove the stale SQL_C_STINYINT/SQL_C_TINYINT entries (listed twice) from the unhandled-C-types comment, since TINYINT columns no longer bind to those types. Addresses review feedback. Signed-off-by: Gustavo Rendón Vélez <22383219+rendonvelez@users.noreply.github.com>
SQLCancel only requests cancellation; when the operation actually aborts depends on the engine reaching a cancellation checkpoint. Note the known cases: Impala's sleep() completes its full wait, and stored procedure calls on Db2 for IBM i are not cancelable. Signed-off-by: Gustavo Rendón Vélez <22383219+rendonvelez@users.noreply.github.com>
…o integration/saturno
Link darwin binaries with -headerpad_max_install_names so that install_name_tool can later add LC_RPATH entries to the prebuilt without overflowing into the __text section. The official v2.5.0 darwin prebuilts shipped with only 64 bytes of padding, which corrupts the binary when consumers patch rpaths.
- Drop the ppc64le job: that runner is not available on this fork and would block the release job forever. - Mark releases as prerelease and remove the npm publish step: this fork only distributes prebuilt binaries via GitHub Releases.
Bump to a distinguishable prerelease version and point binary.host to this fork's GitHub Releases so node-pre-gyp downloads the patched prebuilts instead of the upstream ones.
The node-gyp bundled with npm on Node 20 fails to locate Visual Studio on windows-latest (ERR_CHILD_PROCESS_STDIO_MAXBUFFER in 'find VS'). Mirror the create-release workflow: pin windows-2022 and upgrade npm before building.
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
1 participant
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.
Purpose
Do not merge. This draft PR exists to run the Build workflow against the consolidated integration branch used by saturno while the upstream PRs are pending review:
Contents
feature/query-cancellationandfix/tinyint-negative-valueson top of upstreammain(post v2.5.0).binding.gyp: darwin builds now link with-headerpad_max_install_namesso LC_RPATH entries can be added to the prebuilts without corrupting them (upstream v2.5.0 darwin prebuilts shipped with only 64 bytes of header padding).create-release.yml: ppc64le job and npm publish removed for fork-hosted releases.2.5.0-saturno.1;binary.hostpoints to this fork's releases.This branch will be deleted once IBM merges both PRs and publishes a release containing them.