jonwagner / Insight.Database

Fast, lightweight .NET micro-ORM
Other
861 stars 145 forks source link

Parameter size in sql commands #438

Closed jkl83 closed 4 years ago

jkl83 commented 4 years ago

Is it possible to define input parameter size in Insight Database sql command call? I have query like this:

sqlTransaction.QuerySql(QueryFiles.SomeQuery, new
        {
            some_text_param = paramValue
        });

But this query creates new execution plan in db for each parameter length. Can the size be set? I mean by using the size parameter for the SqlParameter:

public SqlParameter (string parameterName, System.Data.SqlDbType dbType, int size);

DB is MsSql 2012.

Some related info can be found here: ADO.NET: Safe to specify -1 for SqlParameter.Size for all VarChar parameters?

My current version is 5.2.10.

Thanks.

jonwagner commented 4 years ago

You shouldn't need to set the parameter length. For string types, Insight will automatically calculate the length or specify -1 if it is over the limit for varchar(n).

jkl83 commented 4 years ago

@jonwagner, that is exactly the problem. String length is different every time. So insight sets it everytyme to a different size. This causes the sql server to generate new execution plan for each parameter size. I would like to set the size manually, so that every time would be used the same execution plan.

jonwagner commented 4 years ago

Got it. We're definitely not going to expose the size field to callers. That would violate the simplicity rules of the library. But if there's a performance issue, we'll want to track it down and get people the best performance possible.

Can you provide a test case and tools to illustrate the issue so we can track it down? That article is over 7 years old and doesn't provide any details on how to track performance impact.

jkl83 commented 4 years ago

Code:

`namespace UnitTestProject1 { [TestClass] public class UnitTest1 { private const string Query = "select * from example_table where string_column = @name";

    [TestMethod]
    public void Query1()
    {
        var result = GetConnection().QuerySql(Query, new
        {
            name = "ShortString"
        });
    }

    [TestMethod]
    public void Query2()
    {
        var result = GetConnection().QuerySql(Query, new
        {
            name = "ExtraLongStringForLargeParameterSize"
        });
    }

    private SqlConnection GetConnection()
    {
        return new SqlConnection(ConfigurationManager.ConnectionStrings["Dz"].ConnectionString);
    }
}

}`

Execution in profiler:

image

Query plans created: The middle one was made by manually executing query in sql management squdio, so it doesn't count.

image

jonwagner commented 4 years ago

Got it. I'm not seeing that effect for stored proc calls, but it's occurring for sql text commands. We should be able to update the Sql Server providers to automatically fix up the parameters in those cases.

jkl83 commented 4 years ago

Stored procedures have input parameters size declared so it doesn't change. That's why there is always one plan for them.

Thanks.

jonwagner commented 4 years ago

This is fixed in the latest code. I'll do a build after I finish fixing a few build issues.

jkl83 commented 4 years ago

I see that you're setting -1 for all of the command string parameters. According to: https://stackoverflow.com/questions/14342954/ado-net-safe-to-specify-1-for-sqlparameter-size-for-all-varchar-parameters

It is not a good idea.

Maybe use this logic: `if((sqlDbType == SqlDbType.VarChar) || (sqlDbType == SqlDbType.NVarChar)) { m_sqlParam.Size = (sqlDbType == SqlDbType.VarChar) ? 8000 : 4000;

if((value != null) && !(value is DBNull) && (value.ToString().Length > m_sqlParam.Size))
    m_sqlParam.Size = -1;

}`

Inside the GetStringParameterLength method? Set it to 8000 by default, and only if the text is longer, set it to -1?

jonwagner commented 4 years ago

Right... -1 could be treated as max and then get a different plan.

I wonder what happens if we leave it unset. Does the sql driver use this logic or does it calculate the length of the string?

On Jul 30, 2020, at 6:58 AM, jkl83 notifications@github.com wrote:



I see that you're setting -1 for all of the command string parameters. According to: https://stackoverflow.com/questions/14342954/ado-net-safe-to-specify-1-for-sqlparameter-size-for-all-varchar-parameters

It is not a good idea.

Maybe use this logic: `if((sqlDbType == SqlDbType.VarChar) || (sqlDbType == SqlDbType.NVarChar)) { m_sqlParam.Size = (sqlDbType == SqlDbType.VarChar) ? 8000 : 4000;

if((value != null) && !(value is DBNull) && (value.ToString().Length > m_sqlParam.Size)) m_sqlParam.Size = -1;

}`

Inside the GetStringParameterLength method? Set it to 8000 by default, and only if the text is longer, set it to -1?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/jonwagner/Insight.Database/issues/438#issuecomment-666297434, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAMTO5GQQNTEHMLRNO74T6TR6FG5BANCNFSM4MHR4SRA.

jkl83 commented 4 years ago

If it's ADO.NET underneath then according to: https://docs.microsoft.com/en-us/dotnet/api/system.data.sqlclient.sqlparameter.size?redirectedfrom=MSDN&view=dotnet-plat-ext-3.1#System_Data_SqlClient_SqlParameter_Size

If not explicitly set, the size is inferred from the actual size of the specified parameter value.

So basically back to square one :)

More info: https://stackoverflow.com/questions/903651/default-sizes-for-sql-parameters-in-ado-net

jonwagner commented 4 years ago

@jkl83 take a look at the code in DbParameterGenerator.cs

jkl83 commented 4 years ago

Looks good to me. Waiting for release. Thank you!

jonwagner commented 4 years ago

This is updated in v6.3.1