TortugaResearch / Tortuga.Chain

A fluent ORM for .NET
Other
336 stars 22 forks source link

SQL Server Parameter Lengths - SqlServerDataSource.Sql #500

Closed agrinei closed 7 months ago

agrinei commented 1 year ago

This is similar to #402 #414 #499 but for SqlServerDataSource.Sql. It's definitely much harder to solve so it deserves its own issue.

When calling SqlServerDataSource.Sql it doesn't use appropriate column definition (varchar / nvarchar / max length) to set string parameters. It does work when we call SqlServerDataSource.From

[IT USES COLUMN DEFINITION] sqlServerDataSource.From("MY_TABLE_NAME", new { my_column_name = "COLUMN_VALUE" })

[IT DOESN'T USE COLUMN DEFINITION] sqlServerDataSource.Sql("select * from MY_TABLE_NAME", new { my_column_name = "COLUMN_VALUE" })

Versions tested:

Tortuga.Chain.SqlServer v.4.3.0 Tortuga.Chain.SqlServer.MDS v.4.3.0

Grauenwolf commented 1 year ago

The basic problem is that we can't understand raw SQL. Without a full SQL parser that mirrors what SQL Server is doing internally, there is no way to predict what parameter type/size SQL Server would prefer.

The most direct workaround is to simply provide a SqlParameter collection to that function rather than an object parameter. But I can see how that could be inconvenient.

Grauenwolf commented 1 year ago

One option would be to add some assumptions at the data source level. For example,

.DefaultStringType : When the expected string type is unknown, use the indicated type. May be varChar or nVarChar. This will reduce the likelihood of string type conversions in the execution plan. .DefaultStringLength: When the expected string length is unknown, and the provided string is less than DefaultStringLength long, then use DefaultStringLength as the parameter length. This will reduce the frequency of redundant execution plans.

You would also be able to provide these at the Command Builder level, just like we can override strict mode or change a timeout.

TODO: Think really hard about these names.

Grauenwolf commented 1 year ago

Option 2 would be to use reflection on the parameter object to look for a Column attribute.

This would require a change to Anchor, as that's where we cache property metadata.

Note that we'll have to do some parsing, as the type comes to us as a string.

[Column("Description", Order = 2, TypeName = "nvarchar(100)")]
agrinei commented 1 year ago

My first thought was also a full sql parser. But (I think) we could achieve good results with basic checks:

This 'basic sql parser' is kind of slippery so [ParameterTypeGuessing] could be turned on or off at the datasource or command level.

Grauenwolf commented 1 year ago

Setting the default at the data source level.

dataSource = dataSource.WithSettings(new() { DefaultStringType = SqlDbType.NChar });
dataSource = dataSource.WithSettings(new() { DefaultStringLength = 25 });

Setting the default at the command builder level.

var countA = dataSource.Sql(CheckA, CheckParameter1).WithStringLength(50).ToInt32().Execute();
var countA = dataSource.Sql(CheckA, CheckParameter1).WithStringType(SqlDbType.Char).ToInt32().Execute();
var countA = dataSource.Sql(CheckA, CheckParameter1).WithStringType(SqlDbType.VarChar).WithStringLength(-1).ToInt32().Execute();

If you have suggestions for the names/syntax, let me know. Otherwise I can add this to v4.4.

agrinei commented 1 year ago

What about

var countA = dataSource.Sql(CheckA, CheckParameter1, new() { DefaultStringType = SqlDbType.NChar }).ToInt32().Execute(); var countA = dataSource.Sql(CheckA, CheckParameter1, new() { DefaultStringLength = 25 }).ToInt32().Execute(); var countA = dataSource.Sql(CheckA, CheckParameter1, new() { DefaultStringLength = 50, DefaultStringType = SqlDbType.NVarChar }).ToInt32().Execute();

Grauenwolf commented 1 year ago

Better, but still not quite right. I'll think about it some more.

Grauenwolf commented 1 year ago

We can drop the prefix default because that's implied by the parameter and class name.

public SqlCallCommandBuilder<AbstractCommand, AbstractParameter> Sql(string sqlStatement, object argumentValue, SqlServerParameterDefaults defaults)

var countA = dataSource.Sql(CheckA, CheckParameter1, new() { StringType = SqlDbType.NChar }).ToInt32().Execute();
var countA = dataSource.Sql(CheckA, CheckParameter1, new() { StringLength = 25 }).ToInt32().Execute();
var countA = dataSource.Sql(CheckA, CheckParameter1, new() { StringLength = 50, StringType = SqlDbType.NVarChar }).ToInt32().Execute();

I'm still not in love with it, but by using a struct here we aren't paying for memory allocation. And we can cache it in a static/readonly field if we want to make the command look a little cleaner.

var countA = dataSource.Sql(CheckA, CheckParameter1, checkASettings).ToInt32().Execute();
agrinei commented 1 year ago

Sounds great to me! It will solve 99% of the cases.

Grauenwolf commented 1 year ago

What if we just hardcode the max size for the parameters?

https://dba.stackexchange.com/questions/324113/is-there-are-reason-to-not-always-use-varchar8000-for-parameters

agrinei commented 1 year ago

Do you see any possible performance implications of this approach? Even if it doesn't impact the query plan, it could affect memory allocation. I'm just guessing. I've tried but couldn't find anything.

Anyway, I'm ok with this as the fallback if the user doesn't specify max size.

Grauenwolf commented 1 year ago

Nothing concrete yet.

I think I'm going to change the properties at the Data Source level so you can set it separately for varChar and nVarChar. That way people can opt in to 4000/8000.

This won't change the syntax at the command builder level.

Grauenwolf commented 1 year ago

Released in v4.4.0.

agrinei commented 1 year ago

Awesome work! Thank you very much.