search

embulk / embulk-output-jdbc

MySQL, PostgreSQL, Redshift and generic JDBC output plugins for Embulk
Other
88 stars 86 forks source link

Use NVARCHAR(max) for CLOB, instead of TEXT, when creating a table in SQLServer #320

Closed joe-td closed 11 months ago

joe-td commented 1 year ago

Currently, the embulk-output-sqlserver mapped CLOB to TEXT type. This mapping will not work with some Japanese collation. It will display error message like SQL Error [4188] [S0001]: Column or parameter 'colume1' has type 'text' and collation 'Japanese_XJIS_140_CI_AS'. The legacy LOB types do not support UTF-8 or UTF-16 encodings. Use types varchar(max), nvarchar(max) or a collation which does not have the _SC or _UTF8 flags.. Moreover, as TEXT type is deprecated and removed in the future version (https://learn.microsoft.com/en-us/sql/t-sql/data-types/ntext-text-and-image-transact-sql?view=sql-server-2016). So this PR will map CLOB type to NVARCHAR(max) type.

Ref: https://learn.microsoft.com/en-us/sql/relational-databases/collations/collation-and-unicode-support?view=sql-server-2017#Unicode_Defn

dmikurube commented 1 year ago

@joe-td I don't think Sada is looking in the repo anymore. Then I'm having a look...

Hmm, I'm not very familiar with that SQL Server. I'm afraid is that... compatible? If a user has been running embulk-output-sqlserver continuously for the same table with config_diff, can the user continue it even after upgrading the plugin with this change?

joe-td commented 1 year ago

Thanks for your feedback @dmikurube. I'm new to sqlserver too and as my understanding, if the table is already existed then there is no issue. This mapping is only use for creating table step.

dmikurube commented 1 year ago

I'd suggest to change the subject to make it explicit changing only when creating a table. ("data mapping" sounds like to be applied for all the records.)

For example, "Use NVARCHAR(max) for CLOB, instead of TEXT, when creating a table in SQLServer"

dmikurube commented 11 months ago

Thanks for confirming! I'm going to merge this, and release v0.10.3.