Unable to set string parameter to NULL using bind overload

[lexicalunit]

From @AndrewJD79 on July 3, 2017 20:40

Environment

• nanodbc version: 2.12.4, master
• DBMS name/version: Progress/ MSSQL

Actual behavior

Unable to set string parameter to NULL using following bind overload:
void bind( short param_index, T const* values, std::size_t batch_size, bool const* nulls, param_direction direction = PARAM_IN);

The overload eventually calls
void statement::statement_impl::bind_parameter<string::value_type>( bound_parameter const& param, bound_buffer<string::value_type>& buffer)

For some reasons last parameter (parameter length or status indicator) passed into SQLBindParameter is
(buffer.size_ <= 1 ? nullptr : bind_len_or_null_[param.index_].data()

I'm not sure why length_or_status is depending from batch size (buffer.size_). If the line changed to
bind_len_or_null_[param.index_].data()
everything works as expected.

Expected behavior

parameter has to have NULL value

Minimal Working Example

statement stat(conn);

size_t batch_size = 1;
char buffer [] = "";
bool isNull = true;
stat.bind(0, buffer, batch_size , &isNull);

Copied from original issue: lexicalunit/nanodbc#277

[lexicalunit]

From @mloskot on July 3, 2017 20:55

The line (elements <= 1 ? NULL : bind_len_or_null_[param].data())) was introduced in change f024f84 a long while ago.

I'm surprised the issue has not leaked earlier. I wonder if it is Progress/MSSQL specific or we don't have it covered with tests properly.

@lexicalunit, since you made that change, do you recall why buffer.size_ <= 1 (used to be elements)?

---

@AndrewJD79 What Progress do you use? Do you know if it is freely available edition we could run on Travis CI or AppVeyor services for testing?

[lexicalunit]

From @AndrewJD79 on July 3, 2017 21:29

I'm not sure about free Progress but it can be easily reproduced on MSSQL.
I don't think that the issue is driver specific because driver cannot known about NULL value due to nullptr is passed as StrLen_or_IndPtr parameter.
BTW, other types like int, doubles works just fine but they use another overload.

[lexicalunit]

From @mloskot on July 3, 2017 21:42

@AndrewJD79

I'm not sure about free Progress but it can be easily reproduced on MSSQL.

This particular one, yes. I was generally looking for Progress to set it up for nanodbc testing.

I don't think that the issue is driver specific because driver cannot known about NULL value
due to nullptr is passed as StrLen_or_IndPtr parameter.

A driver may misinterpret nullptr. I mean, I've seen weird bugs in SQLite ODBC driver.

I suppose @lexicalunit relied on that particular driver during earlier development of nanodbc.

Anyhow, it needs to be fixed.

[lexicalunit]

Interesting. I think this is probably a bug left over from when I was still attempting to somehow refactor the bind interface, especially for strings, to make it less complicated.

[lexicalunit]

From @mloskot on July 7, 2017 11:52

AFAICT, the problem is with using statement::bind to build a string with null. The bind seems support strings in the very basic case only.
Since we don't cover such 'not meant to happen' usage in tests, that is also why we haven't caught it earlier.

Generally, strings are supposed to be bound using the dedicated family of statement::bind_strings.

@lexicalunit Does it seem correct?

TBH, I'm not sure if there is point in fixing statement::bind shortcomings, before the binding interface overhaul.

[lexicalunit]

From @AndrewJD79 on July 7, 2017 17:23

Actually, I do use bind_strings. Please see stack-trace:

1. nanodbc::statement::statement_impl::bind_parameter(const bound_parameter & param, bound_buffer < char> & buffer)

2. nanodbc::statement::statement_impl::bind_strings(nanodbc::statement::param_direction direction, short param_index, const char * values, unsigned __int64 value_size, unsigned __int64 batch_size, const bool * nulls, const char * null_sentry)

3. nanodbc::statement::bind_strings(short param_index, const char * values, unsigned __int64 value_size, unsigned __int64 batch_size, const bool * nulls, nanodbc::statement::param_direction direction)

[lexicalunit]

From @mloskot on July 7, 2017 21:57

OK. I looked at your MWE code, which says bind