Update SQLServerPlatform default clob type from varchar to nvarchar

[thsmrtone1]

Q	A
Type	bug
Fixed issues	#2323, #1182, #1351

Summary

This PR updates SQLServerPlatform default clob type from varchar to nvarchar. This will ensure text/json/array/object columns use nvarchar(max) instead of varchar(max)

Previous PR was closed due to original author deleting their fork. New PR includes updates to unit tests to account for change.

[derrabus]

Can you elaborate your motivation behind this change? Why is the new column type better than the old one?

What happens if I created a database with the current release of DBAL and ran a schema diff after your change has been merged? Can we cover this migration path with a test?

Also, since you've filed this as a bug: Why do you believe the current behavior is wrong? Why does this change quality as a bugfix?

[mvorisek]

👍 for this PR - it is basically #4987

reasoning/repro is in #4987 (comment) - should be added as a functional string/text test for any/every platform

[derrabus]

Understood. Still, the question about the upgrade path remains. Also, the tests are failing on SQL server.

[thsmrtone1]

Thanks @mvorisek, I hadn't actually seen #4987 in my initial search. Seems the validity of this PR isn't being questioned, but as with your previous PR, the biggest question is the upgrade path. If we leave this PR as-is, the schema diff will show all existing text/json/array/object columns as being the wrong type, and any migrations would attempt to update all existing columns at once. Personally, this would be welcomed in my cases, but I can see that potentially being an issue for other people.

@derrabus Truth is, I don't have any suggestions for how to mitigate this. Has anything similar happened previously that could be used as precedent?

Also, as a side note: I will attempt to fix some of the failing tests now that I know about the functional tests. I wasn't even aware of their existence when developing this PR.

[ausi]

I’m not very familiar with MSSQL, but it seems that using VARCHAR() in combination with a _UTF8 collation can often be a better solution to store unicode characters than using NVARCHAR(). E.g. using Latin1_General_100_CI_AI_SC_UTF8.

UTF-8 support seems to be there since SQL Server 2019.

@thsmrtone1 Other than old SQL Server versions, are there any reasons you would prefer NVARCHAR with UCS-2 or UTF-16 over VARCHAR with UTF-8?

To me it seems that UTF-8 might be a better fit for the PHP ecosystem.

[mvorisek]

I belive NVARCHAR is perfect and consistent with the rest - https://github.com/doctrine/dbal/blob/3.7.0/src/Platforms/SQLServerPlatform.php#L1315. This PR however needs a tests as per discussion above.

[github-actions]

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

[github-actions]

This pull request was closed due to inactivity.