Improving support of nvarchar and nvarbinary on the ODBC driver

[HernanRivasAcosta]

No description provided.

[CLAassistant]

All committers have signed the CLA.

[IngelaAndin]

There are a lot of white space changes it would be good if we could have them in a separate commit to make reviewing easier.

[IngelaAndin]

Should not be updated manually!

[IngelaAndin]

This i not going to work out for the nightly builds! Also it eliminates all Postges test which probably is not a good thing.

[IngelaAndin]

It is not ok to change the basic test setup!

[IngelaAndin]

Nothing happened here for a long time. There is no priority to drive this change from the OTP team but we are willing to accept it if any one is interested in making it work!

[rnowak]

Hi,

The issue involving the large data types specified with a max size has been a growing pain for us lately. I've together with a colleague looked at this patch and there is an off by one error somewhere in calculating the chunking or the buffer. We'll see if we can dig further into it and whatever else we need to do to land this upstream.

Additionally, one of the recent PRs (#7000) for odbcserver causes it to misbehave when used towards a Microsoft SQL Server database, where such data types with max size are specified. Before the recent PR, things worked "fine" as long as one used the ancient {SQL Server} driver -- anything newer such as {ODBC Driver 18 for SQL Server} also causes issues. This PR would probably make that good again.

As a side note, I'll be preparing a PR that allows for multiple result sets when using parameterized queries unless you can think of some reason why that shouldn't be done.

Cc: @micke314 @aronisstav @anders-ramsell-telia

[IngelaAndin]

@rnowak It was many years ago that I worked actively to improve this application. I think not handling multiple result sets is probably a missing feature that was never prioritized. We do apricate that being a separate PR. Any hint on how to set up a working test environment on a modern windows environment would also be greatly apricated, I am afraid ours has gone rotten and Postgress is our only working backend at the moment.

[rnowak]

Just a small status update that this is still on my table, it has just been shifted off constantly due to other prioritization. The hope is to have this done before the end of this year. The merged #7000 in combination with only the ancient {SQL Server} driver being usable right now is a pretty large blocker and is preventing us from upgrading to a more recent Erlang version in addition to being able to migrate to Linux.

Not sure how to best proceed, but perhaps it might be better to close this PR and start afresh when it is time. This PR includes a lot of mixed changes, and ultimately, the chosen approach cannot work with the ODBC drivers towards Microsoft SQL Server in certain scenarios as it goes against specific requirements outlined in the Microsoft documentation. It is illegal to retrieve bound columns after a call to SQLGetData -- it causes empty values to be returned silently.

I have an implementation prepared, but it needs some more love. It changes how variable-sized types are retrieved, and this will hold true for all database kinds, not only Microsoft SQL Server. Ultimately, this should match what the drivers might have performed under the hood otherwise, and hopefully it will have similar performance characteristics. I will try to verify my hopes towards PostgreSQL.

I want to reiterate that the PR as it is currently should not be merged.

[IngelaAndin]

@rnowak You are of course still most welcome to do the work you intended on this one. I fully agree it should not be merged as it is. If you feel it would be easier to make a new PR please do, and when you do we can close this one in favor of the new one.
The reason we changed it was that we are cleaning up after OTP-27 release and we short of gave up hope on the waiting label as we have note heard from you in a long time. I also added the review help wanted to enable more interested people to perhaps come with valuable input, just approving it as it is now is however not helpful to the cause if it being mergeable.