Fix SQLPrepare discarding SQL_ATTR_ROWS_FETCHED_PTR and SQL_ATTR_ROW_STATUS_PTR (#301) - #303
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.
|
Confirmed on Linux with the All three orderings now agree: 4 rows then 2, with Thank you for this one as well; two root-caused fixes with tests in a single evening, on two issues from the same reporter, is generous with your time in a way I do not take for granted. Block-cursor readers can now trust the counters in whichever order they set things up, which is exactly what makes the driver usable from bulk-reading clients. Noted on the (Where this comes from: adbcBridge, an ADBC driver over ODBC that runs Firebird in its compatibility matrix on Linux and Windows, https://github.com/singhpratech/adbcbridge. Its Firebird row and upstream tracker now record this fix.) |
Fixes #301.
The problem
OdbcDesc::setDefaultImplDescrebuilds the implementation row descriptor on every prepare, and its IRD branch also resetSQL_DESC_ROWS_PROCESSED_PTRandSQL_DESC_ARRAY_STATUS_PTRto NULL.sqlPreparecalls it (soSQLExecDirecttoo), as doessetResultSetfor catalog functions. An application that setSQL_ATTR_ROWS_FETCHED_PTR/SQL_ATTR_ROW_STATUS_PTRbeforeSQLPrepareorSQLExecDirect— the usual order — never had them written bySQLFetch: the rows-fetched counter kept whatever it held and the status array stayed untouched, on every fetch including the last partial one. Set after the prepare, both worked. The IPD branch of the same function never resets its own counterparts, which is whySQL_ATTR_PARAMS_PROCESSED_PTRsurvives a prepare and the row-side pointers did not.Statement attributes persist until the statement is freed or the attribute is set again;
SQLPrepareis not supposed to clear them. The reset dates from 2004/2008, so every released version is affected.The fix
Drop the two pointer resets from the IRD branch of
setDefaultImplDesc. The other header fields reset there (headAllocType,headArraySize,headBindOffsetPtr) are ARD-only fields in ODBC terms and nothing reads them from the IRD, so they are left as they were.Tests
tests/test_block_cursor.cpp: four cases over a six-rowUNION ALLwithSQL_ATTR_ROW_ARRAY_SIZE = 4, each asserting the counter (4, then 2), the status array (SQL_ROW_SUCCESS/SQL_ROW_NOROW) and the bound values on both rowsets, thenSQL_NO_DATA.RowsFetchedPtrSetBeforePrepare— also asserts thatSQLGetStmtAttr(SQL_ATTR_ROW_STATUS_PTR)still returns the pointer afterSQLPrepare. Fails on master.RowsFetchedPtrSetBeforeExecDirect— fails on master.RowsFetchedPtrSurvivesHandleReuse— attributes kept acrossSQLFreeStmt(SQL_CLOSE)and a secondSQLExecDirecton the same handle. Fails on master.RowsFetchedPtrSetAfterPrepare— control, passes both ways.Verified on Windows x64 against Firebird 5.0.3 with the Microsoft driver manager. The reporter's program reproduces the output from the issue verbatim on master and prints the correct counters with this branch for all five orderings (before / between / after
SQLPrepare+SQLExecute; before / afterSQLExecDirect). Full suite: 392 ran, 227 passed, 165 skipped, 0 failed.Still open
SQLGetStmtAttr(SQL_ATTR_ROWS_FETCHED_PTR)returnsHYC00:sqlGetStmtAttrhas no case for it (nor forSQL_ATTR_PARAMS_PROCESSED_PTR,SQL_ATTR_PARAM_STATUS_PTR,SQL_ATTR_PARAM_OPERATION_PTR,SQL_ATTR_ROW_OPERATION_PTRand the two*_BIND_OFFSET_PTRattributes), so thedefault:branch answers. Setting them works, and once the statement is prepared the same pointers come back throughSQLGetDescFieldon the IRD (SQL_DESC_ROWS_PROCESSED_PTR,SQL_DESC_ARRAY_STATUS_PTR; before prepare the IRD answersHY091). Only the statement-attribute read-back is missing. Separate change; the test above reads backSQL_ATTR_ROW_STATUS_PTR, which does have a case.fb-cppedition) #283), whose block-fetch test hides the symptom behind anif (rowsFetched > 0). Both will be corrected there on its next rebase.