Fix column-wise parameter arrays reading element 0 for fixed-length C types (#299)

[fdcastel]

Fixes #299.

The problem

SQLBindParameter stores BufferLength as the element stride of a column-wise parameter array (sizeColumnExtendedFetch). The ODBC specification ignores BufferLength for fixed-length C types and applications pass 0 there, so inputParam multiplied the set number by 0 and every parameter set read element 0. SQLExecute returned SQL_SUCCESS, SQL_ATTR_PARAMS_PROCESSED_PTR said N and the status array was all SQL_PARAM_SUCCESS; the table held N copies of the first set. SQL_C_CHAR arrays were stepped correctly because there BufferLength is the element size, which is why the string-only tests never saw it.

The output side has always had the fallback: bindOutputColumn computes the stride from the C type when SQLBindCol gets 0. The input side never did.

A second defect sits on the same path. The executor is chosen at prepare time (execute = &executeStatementParamArray only if the paramset size is already greater than 1). An application that prepares first and sets SQL_ATTR_PARAMSET_SIZE afterwards, which the specification allows, gets executeStatement once: one row, first parameter set, SQL_SUCCESS. pyodbc's fast_executemany does exactly that sequence; against master a five-row executemany stores one row.

Two of the issue's observations are not defects:

• The indicator array is stepped correctly since 0a7cc1f (Fix bug in null-indicator offset calculation, SQLINTEGER->SQLLEN #279). In the report every id collapsed to 1 because the id column was bound the same way, so ORDER BY id over equal keys returned the rows in arbitrary order and the NULL appeared to move.
• SQLRowCount = 1 after an array execute matches the SQL_PARC_BATCH the driver reports for SQL_PARAM_ARRAY_ROW_COUNTS: the count is per parameter set. What is missing is SQLMoreResults stepping to the next set's count; not touched here.

The fix

Three commits.

1. tests: eleven ArrayBindingTest cases were skipped citing a crash in the indicator offset that Fix bug in null-indicator offset calculation, SQLINTEGER->SQLLEN #279 fixed. The skips come out, with the Firebird 6 guard the row-wise cases already carry; ParamOperationPtrSkipRows stays skipped for the reason its row-wise twin gives. Two new cases bind fixed-length C types with BufferLength = 0: the shape from the issue (INTEGER and BIGINT, NULL in the third set) and DOUBLE / DATE / TIMESTAMP.
2. bindInputOutputParam falls back to getConciseSize when the stride is 0, the same switch bindOutputColumn uses. getConciseSize gains SQL_C_GUID, for which it returned the type code (-11), a negative stride on either side.
3. sqlExecute and sqlExecDirect switch to executeStatementParamArray when the paramset size is greater than 1 at execute time and the prepare-time choice was something else.

Tests

On master (35fae3e) with the skips removed, 3 of the 17 active array cases fail: ColumnWisePrepareExecute (sets the paramset size after SQLPrepare) and the two new cases. With this branch all 17 pass; the two OPERATION_PTR cases stay skipped.

A ctypes port of the issue's program, extended with the prepare-then-set-paramset order and pyodbc fast_executemany. Windows x64, Firebird 5.0.3, Microsoft driver manager:

case	master	this branch
column-wise, BufferLength = 0	(1,NULL) (1,10) (1,10) (1,10) (1,10)	(1,10) (2,20) (3,NULL) (4,40) (5,50)
column-wise, BufferLength = sizeof(type)	correct	correct
row-wise	correct	correct
column-wise, SQL_ATTR_PARAMSET_SIZE set after SQLPrepare	(1,10)	correct
pyodbc fast_executemany, five rows	(1,10)	correct

Full suite with this branch, CHARSET=UTF8 and NONE: 390 ran, 235 passed, 155 skipped, 0 failed.

Still open

• SQL_ATTR_PARAM_OPERATION_PTR is ignored (ParamOperationPtrSkipRows and RowWiseWithOperationPtr stay skipped).
• SQLMoreResults after an array execute does not step through the per-set row counts.
• Column-wise binding with SQL_ATTR_ROW_BIND_OFFSET_PTR writes every row of a rowset to the same address #306, the fetch-side twin (column-wise SQLBindCol with SQL_ATTR_ROW_BIND_OFFSET_PTR), is a different branch of fetchData and is not touched here.
• The v5 RFC (v5 - Firebird ODBC Driver Modernization (fb-cpp edition) #283) already carries both fixes.

[singhpratech]

Verified on Linux with this PR's linux-x64-binaries artifact, unixODBC 2.3.12, Firebird 5.0.4, CHARSET=UTF8, against the 3.5.0-rc1 release build in the same session. It fixes #299.

First program from #299 (column-wise with BufferLength=0, the shape the specification prescribes for fixed-length C types), PR build:

column-wise, BufferLength=0                    processed=5 rowcount=1 stored (id,v): (1,10) (2,20) (3,NULL) (4,40) (5,50)
column-wise, BufferLength=sizeof(type)         processed=5 rowcount=1 stored (id,v): (1,10) (2,20) (3,NULL) (4,40) (5,50)
row-wise, SQL_ATTR_PARAM_BIND_TYPE=sizeof(row) processed=5 rowcount=1 stored (id,v): (1,10) (2,20) (3,NULL) (4,40) (5,50)

The release build still stores (1,10) (1,10) (1,10) (1,10) (1,NULL) for the first line.

Second program (seven C types, with and without a NULL in row 3), PR build: every type stores five distinct rows, and the NULL lands in row 3 for all of them. Two things the release build got wrong are both gone: the fixed-length types repeating element 0, and the SQL_C_CHAR case with a NULL indicator in row 3, which used to store [NULL] [a] [bb] [dddd] [eeeee], the indicator array stepped at the wrong stride so the NULL moved to row 1 and ccc vanished. With this PR it is [a] [bb] [NULL] [dddd] [eeeee].

rowcount=1 after the five-row execute is unchanged, which I take to be #311's territory rather than this PR's.

Thank you, again. Three reports, three root-caused fixes with regression tests in the space of a day and a half, and the third one closes the defect that made me turn parameter arrays off for this driver in the first place; once a release carries #308 that switch can go back on, which is the difference between one round-trip per row and one per batch for everyone loading Firebird through ODBC.

On odbc-crusher: I had not seen it and it is exactly the kind of tool I have been wishing existed, an executable version of the specification rather than a reading of it. The compatibility matrix behind these reports runs one workload through 53 ODBC drivers on Linux, macOS and Windows, so I have a ready-made set of drivers to point it at. I will run it across that set and bring anything useful, results included, to your issues page as you suggest.

(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.)