Restore the bind-offset pointer when a parameter-array execute throws (#309) - #313
Open
fdcastel wants to merge 9 commits into
Open
Restore the bind-offset pointer when a parameter-array execute throws (#309)#313fdcastel wants to merge 9 commits into
fdcastel wants to merge 9 commits into
Conversation
Eleven ArrayBindingTest cases were skipped citing a crash in the column-wise indicator offset. That crash was fixed in 0a7cc1f (SQLINTEGER to SQLLEN), so the skips come out. ParamOperationPtrSkipRows stays skipped because SQL_ATTR_PARAM_OPERATION_PTR is still not honoured, and the reactivated cases get the Firebird 6 guard the row-wise ones already have. Two new cases bind fixed-length C types with BufferLength = 0, the usage the ODBC specification describes for those types: one with the shape reported in FirebirdSQL#299 (INTEGER and BIGINT with a NULL in the third parameter set), one across DOUBLE, DATE and TIMESTAMP.
…h is 0 SQLBindParameter stores BufferLength as the element stride of a column-wise parameter array. The ODBC specification ignores BufferLength for fixed-length C types and applications pass 0 there, so every parameter set read element 0: the driver inserted N rows carrying the first set's values and reported SQL_SUCCESS. bindInputOutputParam now falls back to the C type size when the stride is 0, the way bindOutputColumn already does for SQLBindCol. getConciseSize gains SQL_C_GUID, for which it returned the type code (-11) and would have produced a negative stride. Character and binary C types keep BufferLength as their stride, so string arrays behave as before. Fixes FirebirdSQL#299
…fter SQLPrepare The executor is chosen at prepare time from the paramset size in force at that moment. An application that prepares first and sets SQL_ATTR_PARAMSET_SIZE afterwards, which the specification allows and which pyodbc's fast_executemany does, got a single execution of the first parameter set, reported as success. sqlExecute and sqlExecDirect now switch to executeStatementParamArray when the paramset size is greater than 1 at execute time.
ParamOperationPtrSkipRows and RowWiseWithOperationPtr were skipped because the driver does not honour the operation array. Both get the Firebird 6 guard the other array cases carry and run from here on.
The operation array was stored in the APD (SQL_DESC_ARRAY_STATUS_PTR) but executeStatementParamArray never read it, so a set marked SQL_PARAM_IGNORE was executed like any other and an application had no way to skip a row short of rebuilding its arrays. The loop now skips ignored sets, leaves their status at SQL_PARAM_UNUSED and does not count them in SQL_ATTR_PARAMS_PROCESSED_PTR. The status array is filled with SQL_PARAM_UNUSED before the loop, so sets that were never reached read as unused, and status elements are addressed by set number instead of through a moving pointer. The processed count is kept per set instead of being written once after the loop, so it includes a failed set as the specification describes.
Two cases check the row count after an array INSERT (five sets, five) and after an array UPDATE whose sets touch one, three and zero rows (four), and that SQLMoreResults has nothing further. The two SQLGetInfo cases now expect SQL_PARC_NO_BATCH and SQL_PAS_NO_SELECT, the values the driver's behaviour matches.
SQLGetInfo advertised SQL_PARC_BATCH, one row count per parameter set stepped with SQLMoreResults, but the driver kept only the last set's count and SQLMoreResults answered SQL_NO_DATA, so an application saw 1 (or 0 when the last set matched nothing) for a five-row insert. executeStatementParamArray now adds up the update count of every set and sqlRowCount returns that total while the last execute was an array execute; a single execute resets it. SQL_PARAM_ARRAY_ROW_COUNTS reports SQL_PARC_NO_BATCH accordingly. SQL_PARAM_ARRAY_SELECTS reports SQL_PAS_NO_SELECT: a result-set statement with a paramset larger than 1 is not routed to the array executor and runs once with the first set, which neither SQL_PAS_BATCH nor SQL_PAS_NO_BATCH describes.
One case runs a five-set INSERT whose third set violates NOT NULL, checks the return code and SQLSTATE, the processed count (3, the failed set included), the status array (success, success, error, unused, unused), then reuses the same handle for a single-row insert and reads the table back.
executeStatementParamArray points the APD's bind-offset pointer at a local for the duration of the loop and hands the same address to the converter. Both were restored on the normal exit and on the inputParam failure exit, but a server error thrown by executeStatement left them pointing into the dead stack frame; the next execute on that handle read a garbage offset and either crashed or inserted a garbage row. The restore is now a small lambda used on all three exits, the exception path marks the failed set SQL_PARAM_ERROR before rethrowing, and the processed count already includes the failed set since the operation pointer change. Fixes FirebirdSQL#309
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 #309. Builds on #308, #310 and #311: their commits show here until they merge. This PR is the last two commits.
The problem
executeStatementParamArraypoints the APD's bind-offset pointer at a local variable for the duration of the loop and hands the same address to the converter. Both were restored on the normal exit and on theinputParamfailure exit. A server error thrown byexecuteStatement(a NOT NULL violation in one set, say) took a third exit that restored nothing:sqlExecutecaught the exception with the descriptor pointing into the dead stack frame, and the next execute on that handle read a garbage offset. In the issue that was an access violation; on the operation-pointer branch the same run inserted a garbage row.The fix
The restore is a small lambda used on all three exits. The exception path also marks the failed set
SQL_PARAM_ERRORbefore rethrowing; the processed count already includes the failed set since #310.Tests
ServerErrorInsideArray: a five-set INSERT whose third set violates NOT NULL, then the same handle reused for a single-row insert. Against the #311 driver: the failed set staysSQL_PARAM_UNUSEDand the reused handle stores nothing under id 7 (the garbage row of the probe below); on master the same sequence is the access violation from the issue. With this branch:SQL_ERROR/23000, processed 3, statusSUCCESS SUCCESS ERROR UNUSED UNUSED, and the reuse stores(7, 'seven'). Array suite 22 of 22. Full suite,CHARSET=UTF8andNONE: 393 ran, 240 passed, 153 skipped, 0 failed.Probe (ctypes, Windows x64, Firebird 5.0.3, Microsoft driver manager), case 5 of the array probe:
OK OK UNUSED UNUSED UNUSEDOK OK ERROR UNUSED UNUSED(7, 70)(0, 612311711129600)(7, 70)