Bug: RepoDb.SqlServer truncates the content of models' string primary key fields when executing an overload of Insert[All]

[claudiaw797]

Bug Description

Preconditions

• A table with a primary key field of type [N][VAR]CHAR and a length exceeding 30 characters.
• Models to insert into the table, containing ID values that also exceed this limit.

Faulty behavior

After executing any overload of SqlConnection.Insert[All][Async], the models' field values are truncated to 30 characters.

Reason

Internally, the library executes a query containing SELECT CONVERT(NVARCHAR, @IdField) and assigns the result to the model's ID field.
Since the default length for [N][VAR]CHAR when casting or converting is 30, characters beyond that are cut off.

https://learn.microsoft.com/en-us/sql/t-sql/data-types/char-and-varchar-transact-sql?view=sql-server-ver16

"If n isn't specified when using the CAST and CONVERT functions, the default length is 30."

https://learn.microsoft.com/en-us/sql/t-sql/data-types/nchar-and-nvarchar-transact-sql?view=sql-server-ver16

"When n isn't specified with the CAST function, the default length is 30."

Even though the latter quote does not mention CONVERT, the same behavior can be observed for all four data types.

Circumvention

Perform any required setup before inserting the models.
The bug only has an impact if the models are reused after the insert operation.

Proposed simple solution for variable-size string data

Add MAX as the string size for [N]VARCHAR, changing three lines in RepoDb.Resolvers.DbTypeToSqlServerStringNameResolver.Resolve(DbType):

• DbType.String => "NVARCHAR(MAX)",
• DbType.AnsiString => "VARCHAR(MAX)",
• _ => "NVARCHAR(MAX)",

This cannot be applied to [N]CHAR, as MAX is not allowed for fixed-size string data.

Proposed solution

Set the string size in the above locations based on the database schema and/or the model's [Size(n)] attribute instead of using MAX.

RepoDb.DbField.Size currently reflects the value from [sys].[columns].[max_length] (which is twice the string length; the correct value could be retrieved from [INFORMATION_SCHEMA].[COLUMNS].[CHARACTER_MAXIMUM_LENGTH]).
However, this value is dismissed on the way to constructing the insert statement.

Schema and Model:

CREATE TABLE [dbo].[NVarCharTable] (
    [Id] NVARCHAR(36)    NOT NULL,
    [Value] NVARCHAR(36) NULL,
    CONSTRAINT [PK_NVarCharTable] PRIMARY KEY CLUSTERED
    (
        [Id] ASC
    )
) ON [PRIMARY]

public record StringPkTable
{
    //[Size(36)]
    public string Id { get; init; } = string.Empty;
    public string Value { get; init; } = string.Empty;

    public int IdLength => Id.Length;
    public int ValueLength => Value.Length;
}

Library Version:
RepoDb v1.13.1 and RepoDb.SqlServer v1.13.1

[rhuijben]

As ID is not an identity column it shouldn't even retrieve the value from the database after inserting. This issue was [for Sqlserver] already fixed on master before I forked my version of RepoDB.

When writing a testcase for this issue I found that the issue wasn't fixed for Mysql yet, so a fix for that is on its way.

Only server side generated values need to be returned in the entities passed to .InsertAll().

I also identified and fixed the issue with the size retrieval on nchar based fields.

Just using "varchar(max)" as generated value type for string will give a lot of issues in the database. E.g. Sqlserver doesn't allow indexes and certain other operations on such columns.

[rhuijben]

The insert issue is for Sqlserver already fixed in the current version of https://www.nuget.org/packages/AmpScm.RepoDb.SqlServer/
The other fixes described should arrive soon assuming there are no regressions in other parts of RepoDb.

See https://github.com/ampscm/repodb/

[claudiaw797]

Thanks for your answers.

As ID is not an identity column it shouldn't even retrieve the value from the database after inserting.

Yes, you're right! It was kind of surprising to see the primary keys being mutilated by the insert call ;(

Only server side generated values need to be returned in the entities passed to .InsertAll().

That's absolutely correct as well.
However, as long as the generated statement contains [N][VAR]CHAR without a defined size, any value exceeding 30 characters (like GUIDs) will be truncated.

Just using "varchar(max)" as generated value type for string will give a lot of issues in the database. E.g. Sqlserver doesn't allow indexes and certain other operations on such columns.

That's probably true too… I don’t know enough about the inner workings of RepoDB.

See https://github.com/ampscm/repodb/

I ran the tests from commit 1aff2c1 (within my fork of RepoDB) inside ampscm/repodb.
InsertAll[Async] works, but Insert[Async] still truncates the primary keys, and therefore those tests fail.

Anyway, the issue came up while using RepoDB at work. Unfortunately, using AmpScm.RepoDb.SqlServer doesn't seem to be an option.