Process every parameter set during SQLExecute

[singhpratech]

The second half of #582, as offered on #586.

A parameter array stops after the first set on a statement that returns nothing. SQLExecute sends set 0 and leaves the rest to SQLMoreResults, which this driver uses to send one set per call — but an INSERT produces no result sets, so a conforming caller has no reason to call SQLMoreResults at all, and the remaining sets are never sent. A five-set array inserts one row, under SQL_SUCCESS, with no diagnostic.

The change

ODBC has the driver execute the statement once per parameter set during SQLExecute; SQLMoreResults then walks the result sets those executions produced. executeQuery now keeps sending sets while the last one produced no result set, and stops as soon as one does — so the one-result-set-per-SQLMoreResults behaviour that a SELECT with a parameter array relies on (the path #324/#325 built, and what StatementParameterBindingsTest.IntArray/StringArray cover) is unchanged.

Second, smaller thing in the same area: SQL_ATTR_PARAMS_PROCESSED_PTR was assigned next_param_set_idx before the set was sent, while that index still named the set about to go — so it reported one fewer than the number processed. It is now written after the send, where the index equals the count completed. Your TODO about output parameters is kept as it was. Happy to split that into its own commit or drop it if you would rather keep this to one thing.

Verified against a live server

ClickHouse 26.7.5.10, two standalone ODBC programs, driver built from this branch:

case	before	after
INSERT, 5 sets	1 row, params_processed=0	5 rows, params_processed=5
SELECT ?, 3 sets	3 result sets in order, params_processed=2	3 result sets in order, params_processed=3

The second row is the regression guard: the values come back 111, 222, 333 in order across three SQLMoreResults calls on both builds, so lazy sending for result-producing statements still works.

The four tests from #586 also still pass on this build — the reproducer from #582 part 1 reports 0 problems here against 5 on the released 1.5.5.20260810.

One caveat about my local build

I could not run the gtest suite: this machine has no Clang, and with GCC driver/test/result_set_reader.hpp and scalar_functions_it.cpp fail with explicit specialization in non-namespace scope, which Clang accepts as an extension. That is pre-existing and unrelated to this change — the driver itself builds and the standalone programs above exercise the behaviour end to end — but it does mean CI is the first place the existing tests will run against this. Two local, uncommitted tweaks were needed to build at all under GCC (skipping the bundled libc++, and a missing <memory> include in the vendored Poco); neither is in this pull request.

Found through adbcBridge (https://github.com/singhpratech/adbcbridge), a driver for ADBC — Apache Arrow's database connectivity API — that works over any ODBC driver, where a bulk insert is a parameter array and losing four rows in five is silent data loss.

[slabko]

Hi @singhpratech,

Thank you for your PR. This is indeed a pretty common case.

I have a couple of remarks here:

• The comments in the code are very specific to this PR and its example. After this is merged, the comment in Statement::executeQuery will make little sense without the context of this PR. I think it can be much shorter and doesn't require that much justification. Similarly, the comment in Statement::requestNextPackOfResultSets mentions the five-set array, which is hard to follow without example in this PR.
• Also, would you mind adding a simple test case?

Just a heads-up: I merged master into your branch to allow the CI to pass for external contributions.

[slabko]

@singhpratech,

After some testing and reading the documentation, I think this PR requires a couple of additional fixes around error handling. Here I quite https://learn.microsoft.com/en-us/sql/odbc/reference/develop-app/using-arrays-of-parameters?view=sql-server-ver17 section Error Processing.

If an error occurs while executing the statement, the execution function returns an error and sets the row number variable to the number of the row containing the error. It is data source-specific whether all rows except the error set are executed or whether all rows before (but not after) the error set are executed. Because it processes sets of parameters, the driver sets the buffer specified by the SQL_ATTR_PARAMS_PROCESSED_PTR statement attribute to the number of the row currently being processed. If all sets except the error set are executed, the driver sets this buffer to SQL_ATTR_PARAMSET_SIZE after all rows are processed.

The CH ODBC driver indeed does not continue when it faces an error, which is permitted. However it must set the SQL_ATTR_PARAMS_PROCESSED_PTR to "statement attribute to the number of the row currently being processed". This is a bit confusing, so I checked https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlsetstmtattr-function?view=sql-server-ver17 the part about SQL_ATTR_PARAMS_PROCESSED_PTR:

An SQLULEN * record field that points to a buffer in which to return the number of sets of parameters that have been processed, including error sets. No number will be returned if this is a null pointer.

This, I think, one key difference from the current implementation, because if an error occurs on the row 2 (the row numbers start from 1, not 0 - the ODBC style), the SQL_ATTR_PARAMS_PROCESSED_PTR value is currently set 1, but must be set to 2.

Additionally, the same page https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlsetstmtattr-function?view=sql-server-ver17 describes SQL_ATTR_PARAM_STATUS_PTR:

The status values can contain the following values:

SQL_PARAM_SUCCESS: The SQL statement was successfully executed for this set of parameters.

SQL_PARAM_SUCCESS_WITH_INFO: The SQL statement was successfully executed for this set of parameters; however, warning information is available in the diagnostics data structure.

SQL_PARAM_ERROR: There was an error in processing this set of parameters. Additional error information is available in the diagnostics data structure.

SQL_PARAM_UNUSED: This parameter set was unused, possibly due to the fact that some previous parameter set caused an error that aborted further processing, or because SQL_PARAM_IGNORE was set for that set of parameters in the array specified by the SQL_ATTR_PARAM_OPERATION_PTR.

As I understand, if I have a five parameter sets and the second parameter set fails, SQL_ATTR_PARAM_STATUS_PTR must be set to [SQL_PARAM_SUCCESS, SQL_PARAM_ERROR, SQL_PARAM_UNUSED, SQL_PARAM_UNUSED, SQL_PARAM_UNUSED]

To test it I crated a table that permits inserting Int32 in one of the columns but sets another column, by default, to Int16, causing conversion an error if the value I insert into the Int32 column is too large for Int16:

CREATE TABLE BatchInsert
(
    id INTEGER,
    value VARCHAR(255),
    copy_id Int16 DEFAULT accurateCast(id, 'Int16')
)
ORDER BY id

Now, the following query will fail but only if the first parameter in the parameter set is grater than 32767

INSERT INTO BatchInsert (id, value) VALUES (?, ?)

Binding these parameters will cause an error on the parameter set 3:

int ids[] = {1, 2, 40000, 4, 5};
char values ...

The result will be:

SQL_ATTR_PARAMS_PROCESSED_PTR: 2 (must be 3)
SQL_ATTR_PARAMS_PROCESSED_PTR: {SQL_PARAM_SUCCESS, SQL_PARAM_SUCCESS, SQL_PARAM_SUCCESS, SQL_PARAM_SUCCESS, SQL_PARAM_SUCCESS} (must be {SQL_PARAM_SUCCESS, SQL_PARAM_SUCCESS, SQL_PARAM_ERROR, SQL_PARAM_UNUSED, SQL_PARAM_UNUSED})

[singhpratech]

Thank you — you were right on both counts, and the error-path spec quotes were exactly what I needed.

Pushed as c6a9cf8: the comments are cut down to the invariant, and there are two tests, the second being your accurateCast table with {1, 2, 40000, 4, 5}. On that case the driver now returns SQL_ERROR with SQL_ATTR_PARAMS_PROCESSED_PTR = 3 and statuses {SUCCESS, SUCCESS, ERROR, UNUSED, UNUSED}.

The all-SUCCESS you saw came from getParamsBindingInfo marking a set successful at binding time, before the server had seen it; that is gone. Each set is now marked in requestNextPackOfResultSets once the server has answered, around a small sendParamSet() split off from the HTTP send, so the sets SQLMoreResults sends are counted the same way as the ones SQLExecute sends — a three-set SELECT walked through SQLMoreResults ends at processed = 3 with three successes.

One honest note on verification: I could only build the test suite under GCC with local workarounds for the in-class template <> specialisations in the test headers, so CI is the first place it runs as you build it. Locally, 391 of 392 pass against ClickHouse 26.7.5.10; the one failure is AuthenticationTest.PasswordEncoding, which needs CREATE USER privilege my fixture user lacks and fails the same way on the released 1.5.5.

[singhpratech]

Thank you for the review and the merge, slabko. The error-path questions made this a better change than the one I opened with, and the accurateCast case is now the test that pins it down. Both halves of #582 are on master now, which means adbcBridge (https://github.com/singhpratech/adbcbridge) can switch its ClickHouse ingest back to parameter arrays as soon as a release carries them. I'll re-run the reproducers on that tag and report back.