Repository navigation
Clear the connection error after SQLError reports it (fixes #102) - #104
Open
singhpratech wants to merge 1 commit into
Open
singhpratech wants to merge 1 commit into
singhpratech wants to merge 1 commit into
Conversation
SQLError takes no record number: by the ODBC 2 contract each call hands back
the next diagnostic and the driver forgets it, so a caller looping until
SQL_NO_DATA_FOUND terminates. OPENSEARCHAPI_ConnectError read the error with
CC_get_error() and returned SQL_SUCCESS without clearing it, so every call
returned the same record.
unixODBC's SQLDriverConnect(W) prefers SQLError(W) over SQLGetDiagRec(W) when
the driver exports it, and collects diagnostics in a
do { } while (SQL_SUCCEEDED(ret)) loop. With the error never cleared that loop
does not end: any connect that cannot succeed - a closed port, useSSL=1
against a plaintext cluster, an unknown auth= value - leaves the process at
100% CPU rather than returning SQL_ERROR.
EN_get_error() already clears the environment's error for this reason, which
is why OPENSEARCHAPI_EnvError terminates. This does the same for the
connection, in ConnectError rather than in CC_get_error(), because
SQLGetDiagRec must stay repeatable. The clear happens after the last use of
msg, which points at __error_message, and the cbErrorMsgMax == 0 retry path is
left alone so that a caller sizing its buffer can still re-fetch the record.
Signed-off-by: singhpratech <42719720+singhpratech@users.noreply.github.com>
singhpratech
requested review from
GumpacG,
MaxKsyunz,
Swiddis,
YANG-DB,
Yury-Fridlyand,
acarbonetto,
anirudha,
dai-chen,
derek-ho,
joshuali925,
kavithacm,
mengweieric,
penghuo,
ps48,
seankao-az and
vamsimanohar
as code owners
September 24, 2026 05:17
singhpratech
added a commit
to singhpratech/adbcbridge
that referenced
this pull request
Sep 26, 2026
…s parameter-array half (#150) opensearch-project/sql-odbc#104 (ConnectError never clears its error, so a failed connect spins forever) and #105 (a character value asked for as a datetime came back as today's date) were opened 2026-09-24. ClickHouse/ clickhouse-odbc#588 covers the second half of #582; the maintainer's review asked for the ODBC error-path contract, which was added the same day and verified on a driver built from the branch. None of the three has landed. The rows say what each PR does and how far it was verified, which for the OpenSearch pair is a syntax check only.
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.
Fixes #102.
SQLErrortakes no record number: by the ODBC 2 contract each call hands back the next diagnostic and the driver forgets it, so that a caller looping untilSQL_NO_DATA_FOUNDterminates.OPENSEARCHAPI_ConnectErrorread the error withCC_get_error()and returnedSQL_SUCCESSwithout clearing it, so every call returned the same record.unixODBC's
SQLDriverConnect(W)prefersSQLError(W)overSQLGetDiagRec(W)when the driver exports it, and collects diagnostics in ado { } while (SQL_SUCCEEDED(ret))loop. With the error never cleared that loop does not end, so any connect that cannot succeed — a closed port,useSSL=1against a plaintext cluster, an unknownauth=value — leaves the process spinning at 100% CPU instead of returningSQL_ERROR. The issue has thestraceevidence: 31,948 writes to the log in 20 seconds, all of them the sameConnectErrorentry.The change
Four lines in
src/sqlodbc/environ.c. There is already a precedent in the same file:EN_get_error()clears the environment's error after handing it out, which is whyOPENSEARCHAPI_EnvErrorterminates. This does the same for the connection.Three details worth stating, since they are the reasons it is written this way rather than more simply:
ConnectError, not inCC_get_error().SQLGetDiagRecmust stay repeatable by record number, and it reaches the same data; clearing inside the getter would break it.msg.CC_clear_error()frees__error_message, which is exactly whatCC_get_error()handed back inmsg, so clearing earlier would leave a dangling pointer.cbErrorMsgMax == 0path is left alone. That is the buffer-sizing retry, which deliberately re-sets the error number so the caller can fetch the record properly on the next call; clearing there would lose it.What I could and could not verify
gcc -fsyntax-onlyon the edited translation unit is clean and produces no new diagnostics against the unpatched baseline. I could not build or run the suite:src/sqlodbc/CMakeLists.txtadds/usr/src/linux-headers-5.0.0-27/includeto the include path, which does not exist on a current distribution, and the unit tests want vcpkg, the AWS SDK and a live cluster. So this is a reasoned fix rather than a locally tested one, and I would rather say that than imply otherwise — please let CI judge it.The reproduction in #102 needs no index and no fixture: point
SQLDriverConnectWat a closed port and it currently never returns. That is the behaviour to check before and after.On the DCO
The commit is signed off as
singhpratech, which is the identity all my other upstream contributions use. I seeCONTRIBUTING.mdasks for a real name; if that is a blocker I am happy to amend the commit to sign off as Prateek Singh with the same address — just say so and I will force-push the change.This came up while running OpenSearch through adbcBridge (https://github.com/singhpratech/adbcbridge), a driver for ADBC — Apache Arrow's database connectivity API — that works over any ODBC driver: it opens connections through the driver manager, so a failed connect that never returns is a hang rather than an error.