Reducing the number of queries created with Decimal data types

[bsstahl]

Note: The goal of this issue is to determine if I should submit a pull-request that I think will make the development experience better when facing one particular problem. My understanding of best-practice is to open an issue first to discuss before simply submitting a PR. I have searched StackExchange and this repository for anything that might already have been done relating to this issue and was unable to find anything. If this has already been addressed, or if I have misunderstood something, please let me know.

As I understand it today, the default behavior for decimal (numeric) data types is for Dapper to submit queries using the smallest possible data type that the data will fit in. Thus, if the value is 123.45, it will be submitted in the query as decimal(3,2) assuming the type has not been explicitly specified. This is a very reasonable default, but does result in what is ultimately different queries being submitted to the DB depending on the values of the parameters.

These different queries are not usually a problem, but can make analysis of those queries more difficult, can cause some very small performance impact since more query plans need to be generated, and in those rare situations where the query plan needs to be pinned, can make that task much more difficult since there can be hundreds of queries depending on the variability of the data.

This problem is solvable by explicitly giving Dapper a data type that matches the column's data type in the database. This way, the query will always be submitted using that type, and only 1 query will be created regardless of how many times the query is executed and the variability of the data. Often, that looks something like this:

    var parameters = new DynamicParameters();
    parameters.Add("decimalParameter1", valueForParameter1, DbType.Decimal, ParameterDirection.Input, 5, 9, 2);
    conn.Execute(query, parameters);

The 5 that preceeds the precision and scale values (9,2) in the parameters.Add(...) method represents the size of the parameter value, and can be calculated using this code block:

    // Values from Transact-SQL Reference > Numeric > decimal & numeric
    // https://docs.microsoft.com/en-us/sql/t-sql/data-types/decimal-and-numeric-transact-sql?view=sql-server-ver15
    int result = 5;
    if (precision > 28) result = 17;
    else if (precision > 19) result = 13;
    else if (precision > 9) result = 9;

The resulting syntax is not horrible by any stretch of the imagination, but it certainly could be cleaner, especially when there are a number of decimal data types in the same query. Also, the functionality in the code block above that calculates the size should probably be encapsulated somewhere so it doesn't have to proliferate. It makes sense to me that this code should live in the Dapper project if possible.

Looking through the project, I see that there is a DbString object that encapsulates some of the complexities of using Dapper with string data types. Following this pattern, I created a similar DbDecimal object that also implements SqlMapper.ICustomQueryParameter and allows the developer to specify the precision and scale values on constructions. This creates queries that are strongly typed, while allowing the query syntax to look something like this:

    var parameters = new DynamicParameters();
    parameters.Add("decimalParameter1", new DbDecimal(9, 2, valueForParameter1));
    conn.Execute(query, parameters);

Is this DbDecimal object something that people believe belongs in the core project? Is there a better way to simplify this story for developers? I look forward to your input.

FWIW: If we decide to go forward with this and do a PR, we'll need to decide what defaults (if any) there should be for the precision and scale values.

[mgravell]

Thought experiment: we should generalize the concept of DbString to a more generic DbValue (naming is hard) that is a read-only struct that encapsulates all of the available common features.

Would that be sensible? Bad? Thinking aloud here.

On Thu, 10 Sep 2020, 22:03 Barry Stahl, notifications@github.com wrote:

Note: The goal of this issue is to determine if I should submit a pull-request that I think will make the development experience better when facing one particular problem. My understanding of best-practice is to open an issue first to discuss before simply submitting a PR. I have searched StackExchange and this repository for anything that might already have been done relating to this issue and was unable to find anything. If this has already been addressed, or if I have misunderstood something, please let me know.

As I understand it today, the default behavior for decimal (numeric) data types is for Dapper to submit queries using the smallest possible data type that the data will fit in. Thus, if the value is 123.45, it will be submitted in the query as decimal(3,2) assuming the type has not been explicitly specified. This is a very reasonable default, but does result in what is ultimately different queries being submitted to the DB depending on the values of the parameters.

These different queries are not usually a problem, but can make analysis of those queries more difficult, can cause some very small performance impact since more query plans need to be generated, and in those rare situations where the query plan needs to be pinned, can make that task much more difficult since there can be hundreds of queries depending on the variability of the data.

This problem is solvable by explicitly giving Dapper a data type that matches the column's data type in the database. This way, the query will always be submitted using that type, and only 1 query will be created regardless of how many times the query is executed and the variability of the data. Often, that looks something like this:

var parameters = new DynamicParameters(); parameters.Add("decimalParameter1", valueForParameter1, DbType.Decimal, ParameterDirection.Input, 5, 9, 2); conn.Execute(query, parameters);

The 5 that preceeds the precision and scale values (9,2) in the parameters.Add(...) method represents the size of the parameter value, and can be calculated using this code block:

// Values from Transact-SQL Reference > Numeric > decimal & numeric // https://docs.microsoft.com/en-us/sql/t-sql/data-types/decimal-and-numeric-transact-sql?view=sql-server-ver15 int result = 5; if (precision > 28) result = 17; else if (precision > 19) result = 13; else if (precision > 9) result = 9;

The resulting syntax is not horrible by any stretch of the imagination, but it certainly could be cleaner, especially when there are a number of decimal data types in the same query. Also, the functionality in the code block above that calculates the size should probably be encapsulated somewhere so it doesn't have to proliferate. It makes sense to me that this code should live in the Dapper project if possible.

Looking through the project, I see that there is a DbString object that encapsulates some of the complexities of using Dapper with string data types. Following this pattern, I created a similar DbDecimal object that also implements SqlMapper.ICustomQueryParameter and allows the developer to specify the precision and scale values on constructions. This creates queries that are strongly typed, while allowing the query syntax to look something like this:

var parameters = new DynamicParameters(); parameters.Add("decimalParameter1", new DbDecimal(9, 2, valueForParameter1)); conn.Execute(query, parameters);

Is this DbDecimal object something that people believe belongs in the core project? Is there a better way to simplify this story for developers? I look forward to your input.

FWIW: If we decide to go forward with this and do a PR, we'll need to decide what defaults (if any) there should be for the precision and scale values.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/StackExchange/Dapper/issues/1536, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMGQSKRY4NR5Y3ZYODLSFE5LFANCNFSM4RF3VONA .

[bsstahl]

Interesting thought @mgravell. It may very well be true that there are other data types that would benefit from having such an implementation, especially since it seems like the majority of the "special" functionality deals with the size parameter so there may be a way to generalize it more.

As to whether or not it is the best thing to do, my biggest concern is that the more general we make this implementation, the more it becomes like the existing DynamicParameters implementation we already have. Without a bunch of conditionals to handle the differences in types, we'd have to delegate the unique functionality somehow, either with abstract classes, which would seem to be only slightly different from creating separate objects for each type that needs it, or some delegation mechanism to allow each user to override the needed functionality, little different from what exists today.

I may be missing something. Perhaps if I had a 3rd example of special functionality. Is there something with date processing for example that might cause us to need a DbDate object? Perhaps with Guids? I haven't seen any such things to this point but that certainly doesn't mean they don't exist.

[bsstahl]

Does the lack of responses to the question of whether or not I should submit a PR for this indicate that I should do so, or that if I did so it would likely be ignored? I can always create a separate package for my teams to use when they need this functionality, it just seems to me like others might benefit from it as well. I'm OK with whatever is decided though.

[bsstahl]

I was able to figure out that despite the Size parameter being required on the constructor if you specify Precision and Scale, it is not actually needed. Queries created without a Size, but with Precision and Scale perform as expected. This takes some of the usefulness out of a DbDecimal object since it doesn't need to hold functional code. It is now really just syntactic sugar. However, having tried the syntax in a number of ways I am still convinced that adding this object has value since the code is considerably cleaner with it under circumstance like I described. As a result, I will be submitting a PR to add this object (and some unit tests for it) to this project.