fskorgen opened a new issue, #8221:
URL: https://github.com/apache/hop/issues/8221

   ### Apache Hop version?
   
   2.19
   
   ### Java version?
   
   21
   
   ### Operating system
   
   Windows
   
   ### What happened?
   
   **Affected:** 2.19.0 and earlier.
   
   `MsSqlServerDatabaseMeta.getFieldDefinition()` emits `VARCHAR(n)` for every 
Hop String shorter than
   8000 characters and `TEXT` for longer Strings. It does not inspect the 
original JDBC type:
   
   ```java
   case IValueMeta.TYPE_STRING:
     if (length < getMaxVARCHARLength()) {
       retval += length > 0 ? "VARCHAR(" + length + ")" : "VARCHAR(100)";
     } else {
       retval += "TEXT";
     }
   ```
   
   A SQL Server `nvarchar(20)` column is read as Hop String with original type 
`Types.NVARCHAR`, but
   the generated definition is `VARCHAR(20)`. Any Hop String with metadata 
length at or above 8000
   becomes `TEXT`, regardless of whether the original JDBC type was 
national-character data. The native
   SQL Server meta inherits the same method.
   
   This breaks a metadata round trip in two ways:
   
   - `VARCHAR` cannot represent characters outside the column collation's code 
page, so an existing
     Unicode column can become lossy when recreated. SQL Server uses 
`NCHAR`/`NVARCHAR` for
     [UTF-16 Unicode 
data](https://learn.microsoft.com/en-us/sql/t-sql/data-types/nchar-and-nvarchar-transact-sql).
   - Microsoft states that [text, ntext and image will be 
removed](https://learn.microsoft.com/en-us/sql/t-sql/data-types/ntext-text-and-image-transact-sql)
     and recommends `VARCHAR(MAX)`, `NVARCHAR(MAX)` and `VARBINARY(MAX)` 
instead.
   
   There is also a query-performance consequence. Microsoft's JDBC driver
   [sends String parameters as Unicode by 
default](https://learn.microsoft.com/en-us/SQL/connect/jdbc/reference/setsendstringparametersasunicode-method-sqlserverdatasource),
   and SQL Server gives `NVARCHAR` higher type precedence than `VARCHAR`. 
Comparing that parameter
   with a generated `VARCHAR` key can force conversion of the indexed column; 
the driver documentation
   explicitly notes the implicit-conversion overhead for `VARCHAR`/`CHAR` 
columns.
   
   ### Steps to reproduce
   
   1. Create a SQL Server table with an `nvarchar(20)` column and an 
`nvarchar(max)` column.
   2. Read it and generate the table DDL from the resulting row metadata.
   
   **Expected:** `NVARCHAR(20)` and `NVARCHAR(MAX)`.
   **Actual:** `VARCHAR(20)`, and `TEXT` for the long one.
   
   ### Suggested fix
   
   Preserve `Types.NCHAR`, `Types.NVARCHAR` and `Types.LONGNVARCHAR` as 
`NCHAR`, `NVARCHAR` and
   `NVARCHAR(MAX)` when original-column metadata is available. Replace legacy 
`TEXT` with
   `VARCHAR(MAX)` for non-national Strings. If new Hop String fields should 
remain `VARCHAR` by default
   for compatibility, expose an explicit SQL Server connection option for 
Unicode DDL rather than
   discarding the type of columns that were actually read. Observe SQL Server's 
declared limits — 8000
   bytes for `VARCHAR(n)` and 4000 byte-pairs for `NVARCHAR(n)` — before 
switching to `(MAX)`.
   
   
   ### Issue Priority
   
   Priority: 2
   
   ### Issue Component
   
   Component: Database


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to