Repository navigation
Conversation
GetNextResultSet called the public SQLNumResultCols. In a Linux shared object that call is interposable and binds to the driver manager's export, which doesn't know the driver's own statement handle and fails, so GetNextResultSet returned before popping a page: SQLFetch stopped after the first FetchSize rows with SQL_NO_DATA. Call the internal OPENSEARCHAPI_NumResultCols, which is what macOS and Windows already reached through the same call. Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
When a later cursor page failed (an HTTP error such as a proxy's 502, a dropped connection, an unparseable body), SendCursorQueries stopped and GetNextResultSet took the empty queue for the end of data: SQLFetch returned SQL_NO_DATA on a truncated result set. A failure on the second page truncated it to the first; one on a later page left the server cursor id set, so the fetch loop spun forever. A NULL response returned without clearing m_is_retrieving, so PopResult waited forever. The cursor thread now records its error in a dedicated, mutex-guarded member (m_error_details is read unlocked by the app thread, so the thread no longer writes it), always clears m_is_retrieving, and makes a best-effort close of the server cursor after an error response. GetNextResultSet returns SQL_ERROR when a page failed and keeps the cursor id, and SQLFetch reports it, with the status and bounded body, once the rows received before it are fetched: SQLSTATE 08S01 for a connection failure, HY000 otherwise. The first page's cursor id is kept so a failed second page has a cursor to report on. m_is_retrieving is now atomic and set before the thread starts, so a PopResult that wins the race doesn't see the end of data either. The next ExecDirect clears the error. Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Swiddis
changed the title
mengweieric
left a comment
mengweieric
left a comment
There was a problem hiding this comment.
Checked both fixes against the artifact and the code paths.
The Linux interposition claim holds: the built libsqlodbc.so exports SQLNumResultCols with default visibility and the dynamic section has no SYMBOLIC flag, so a call from inside the library goes through the PLT and binds to whichever DSO the loader saw first, which is the driver manager when the app links unixODBC. Calling OPENSEARCHAPI_NumResultCols directly sidesteps that, and the head has no remaining undefined reference to SQLNumResultCols. Worth knowing this pattern applies to any other internal call into a public SQL* entry point on Linux.
The cursor error path is well put together. The separate mutex guarded member is the right choice given the app thread reads m_error_details unlocked, m_is_retrieving as atomic and set before the thread starts closes the PopResult race, and every exit from SendCursorQueries now reaches the m_is_retrieving reset so PopResult cannot wait forever. In ExtendedFetch, a break with rows_left falls through to return the rows already received, and because the server cursor id is kept, the next fetch re enters GetNextResultSet with report_error true and returns SQL_ERROR with the status and bounded body. The SQLSTATE split (08S01 for a connection failure, HY000 otherwise) matches what the error types mean. SendCloseCursorRequest wrapped in its own try is necessary now that it runs on the detached thread.
All 44 ut_conn tests pass on both e2e legs, including the five new TestCursorError cases, and the two pagination tests still pass. One thing to be aware of: the e2e job runs ut_conn, which drives OpenSearchCommunication directly, so the SQLNumResultCols fix in GetNextResultSet is not exercised by CI. The itodbc suites would cover it once they build on unixODBC. LGTM.
Description
2 bugfixes related to pagination: fix fetching beyond page 1 overall, and fix partial failures when a page at N>1 fails.
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.