Apply the bind offset to column-wise rowsets instead of taking the row-wise path (#306) - #315
Open
fdcastel wants to merge 5 commits into
Open
Conversation
…Prepare OdbcDesc::setDefaultImplDesc rebuilds the implementation row descriptor on every prepare and, while doing so, reset SQL_DESC_ROWS_PROCESSED_PTR and SQL_DESC_ARRAY_STATUS_PTR to NULL. An application that set SQL_ATTR_ROWS_FETCHED_PTR / SQL_ATTR_ROW_STATUS_PTR before SQLPrepare or SQLExecDirect never had them written by SQLFetch, so a block cursor could not tell how many rows the last rowset held. The same attributes set after the prepare worked, and the parameter descriptor never reset its own counterparts. Statement attributes persist until the statement is freed or the attribute is set again; SQLPrepare must not clear them. Reported in FirebirdSQL#301.
Three cases fail on master: SQL_ATTR_ROWS_FETCHED_PTR and SQL_ATTR_ROW_STATUS_PTR set before SQLPrepare, before SQLExecDirect, and kept across SQLFreeStmt(SQL_CLOSE) on a reused handle. The fourth sets them between SQLPrepare and SQLExecute and is the control that already passed.
Eight more block-cursor cases: SQLFetchScroll(SQL_FETCH_NEXT) and SQLExtendedFetch on a forward-only cursor, a rowset larger than the result set, row-wise binding, a static scrollable cursor in every orientation, and the two catalog paths (SQLTables results are static cursors, SQLGetTypeInfo results are not). Each asserts the rows-fetched counter, the status array and the bound values on every rowset, and a zero counter on SQL_NO_DATA. ColumnWiseBindingWithOffset is skipped with the reason: any bind-offset pointer sends the statement down the row-wise fetch path, which steps by SQL_ATTR_ROW_BIND_TYPE per row, and that is 0 for column-wise binding, so every row of the rowset lands on the same address.
…fetch paths ColumnWiseBindingWithOffset comes out of its skip, and a static-cursor variant is added; the body moves into a fixture helper the two share. Six rows, rowset 4, SQLBindCol(SQL_C_SLONG, values, 0, indicators) with an offset of 8 * sizeof(SQLLEN) bytes, then a second rowset with the offset back at zero; every shifted slot, every unshifted slot and both indicator arrays are checked.
…w-wise path getSchemaFetchData() selected the row-wise fetch branch whenever a bind-offset pointer was set, not only for row-wise binding. That branch advances one byte offset per row by SQL_ATTR_ROW_BIND_TYPE, which is 0 for column-wise binding, so every row of a rowset was written to the same address: the bound address plus the offset. The row-wise branch is now taken only when the bind type is not SQL_BIND_BY_COLUMN. The column-wise branch reads the application's offset once per fetch and passes the row number and the offset to returnDataFromExtendedFetch, which computes offset + elementSize * row for the data and starts the indicator offset at the same base; the row counter no longer travels through the descriptor's pointer. Same change in fetchData and in the static-cursor twin. Fixes FirebirdSQL#306
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.
Fixes #306. Builds on #303 and #305: their commits show here until they merge. This PR is the last two commits.
The problem
getSchemaFetchData()sent a statement down the row-wise fetch branch whenever a bind-offset pointer was set, not only for row-wise binding. That branch advances one byte offset per row bySQL_ATTR_ROW_BIND_TYPE, which is 0 for column-wise binding, so every row of a rowset landed on the same address: the bound address plus the offset. The rows-fetched counter and the status array said four rows; the application's arrays held the last one.The column-wise branch could not simply be selected instead: it used the aliased
headBindOffsetPtras its row counter and never added the application's offset.The fix
The row-wise branch is taken only when
SQL_ATTR_ROW_BIND_TYPEis notSQL_BIND_BY_COLUMN. The column-wise branch reads the application's offset once per fetch and passes the row number and the offset toreturnDataFromExtendedFetch, which now computesoffset + elementSize * rowfor the data and starts the indicator offset at the same base. The row counter no longer travels through the descriptor's pointer. Same change infetchDataand in the static-cursor twin.Tests
BlockCursorTest.ColumnWiseBindingWithOffsetfrom #305 comes out of its skip, and a static-cursor variant is added; the body moves into a fixture helper the two share. Shape: six-rowUNION ALL, rowset 4,SQLBindCol(SQL_C_SLONG, values, 0, indicators), offset 8 ×sizeof(SQLLEN)bytes, then a second rowset with the offset back at zero; every shifted slot, every unshifted slot and both indicator arrays are checked.Against the #305 driver both cases fail (all four rows land on the first shifted slot, the other slots stay untouched); with this branch the block-cursor suite is 13 of 13. Full suite with this branch,
CHARSET=UTF8andNONE: 401 ran, 236 passed, 165 skipped, 0 failed.