Pass correct buffer size to SQLGetData.

[jon-v]

I was getting spurious null bytes in the middle of any CLOB longer than 1023 bytes returned from result::get, when using SQL Server ODBC Driver 11 on Windows 8. This PR fixes that.

However, I just realized that I'm basically reverting part of this commit:

8dbfc45

Looking at the ODBC documentation it seems clear that the correct usage is to pass the actual length of the buffer, but presumably the change in the above commit was made for a reason. Is it possible CLOBs and BLOBs need different handling here?

EDIT: Yes, CLOBs have the null byte while BLOBs don't, so with the current behavior of returning binary data as string, an additional check was needed to avoid removing a byte of real binary data. The commit has been amended accordingly.

[mloskot]

Are you using the latest master? Specifically, with #129 and #130?
With the latter, result_impl::get_ref_impl<string_type> should no longer be called for SQL_BINARY data - it makes little sense to pass stream of bytes via nanodbc::string_type.

In fact, case SQL_C_BINARY could be removed from the switch in result_impl::get_ref_impl. What do you think @lexicalunit? - I scratch that, it might be useful, see my later comment with the three bullet points.

---

Please use git push -f yourremote yourbranch to update the PR, so your PR has single commit.

[mloskot]

Yes, that's what I thought, the Travis failure, but still there is no need for two commits if you do:

git co yourbranch
... # edits
git commit --amend
git push -f yourremote yourbranch

It is quite important to keep the history clean.

[mloskot]

Regarding your note on the buffer_length value, I think you are correct.

https://msdn.microsoft.com/en-us/library/ms715441.aspx

SQLGetData truncates the data to BufferLength less the length of a null-termination character.
It then null-terminates the data. If the length of binary data exceeds the length of the data buffer,
SQLGetData truncates it to BufferLength bytes

[jon-v]

There, I've combined the two commits, and the blob test should pass now. Though as you say, fetching binary data to a std::string seems like the wrong test to be doing.

[mloskot]

Though as you say, fetching binary data to a std::string seems like the wrong test to be doing.

I said to nanodbc::string_type which might be std::u{16|32}string or std::wstring. Fetching to std::string is not that bad idea - that's why case SQL_C_BINARY in the corresponding result_impl::get_ref_impl does not hurt:

• Typically, you want to fetch binary datay into vector<uint8_t> - there is dedicated overload of get_ref_impl.
• Assuming you build without Unicode, you may also want to nanodbc::string_type (aliased to std::string) - then the case SQL_C_BINARY in result_impl::get_ref_impl<string_type> is helpful.
• If, however, you build with Unicode and you still want to fetch into nanodbc::string_type - you can, but you are left with potential problems alone - nanodbc won't prevent you to shoot yourself in foot :)

[mloskot]

LGTM

[lexicalunit]

👍

[mloskot]

@jon-v Thanks for the patch.