On SQL parameter type and size

[Oskar Berggren]

Hi,

In regards to the current pull request about parameter size issues, https://github.com/nhibernate/nhibernate-core/pull/480, I agree with Gunnar's question in the PR discussion, namely, that we should clarify the goal.

I think from the start, no drivers set parameter sizes, making ADO.Net default the size to the size of the actual value - a good thing. This turned out to be a problem on MSSQL, where it would generate excessive cached query plans. As an optimization, the MSSQL driver was changed to set explicit parameter sizes. This introduced a horrifying bug where NHibernate could lose data on INSERT, and yield wrong query results, due to silent data truncation.

The PR mentions size of parameters for the LIKE operator, but I've wondered the last couple of day if this really needs to be specific to just that operator. As mentioned elsewhere we can run in to the same issues with e.g. EQ and LT. Granted, oversized parameters are less useful for these operators, but can occur. I suppose this is similar to Gunnar's question, regarding defining the goal.

In any case, if the solution to this works the same for all operators the code should be simpler than special-casing LIKE.

Requested goals:

1) Parameters that are used with columns mapped as AnsiString should be typed as AnsiString. Corresponding for other types.

2) The driver should be able to set a specific size for a parameter to cope with specific database engine requirements. There are some indications that we would like to avoid extremely large parameter sizes when not required, though clear evidence of performance impact is missing.

3) As before, NHibernate should never silently truncate data in a way that would lose data or yield wrong query results.

My understanding is that the above goals have the following implications:

A) For string data, we should use AnsiString or String as deduced from the type of the other operand.

B) For string data, the length of the type should be at least the length of the current value. (The driver may further round this up where useful).

C) For other types, it should be the type of the other operand, where that can be deduced from the expression.

Regarding (B), I'm currently unclear if this will have repercussions for the LINQ query plan cache. Options to this might be to define the type with a length x times the mapped size, and then provide a possibly slower code path for the few cases where a larger size is needed. Or we could remove size data (use max size), and instead make sure the driver have access to the current value at some point, so that it may adjust the parameter size as it sees fit.

/Oskar

 

[Oskar Berggren]

Btw, I think due to BinaryArithmeticOperatorNode also setting the parameter node type based on the type of the other operand, I suspect queries such as

  "select e.col + @param from ..."

would also not work if the value of the parameter is longer than the mapped size of col.

/Oskar
 

[Gunnar Liljas]

"A) For string data, we should use AnsiString or String as deduced from the type of the other operand."

Yes, this is paramount. In reality, if a AnsiStringType column reference is used in an expression and not explicitly cast to StringType, it should control the type of any affected operands, unless they are also column references.

"B) For string data, the length of the type should be at least the length of the current value. (The driver may further round this up where useful)."

Yes, that's sensible. Unless the length is explicitly set, it should be deduced from the value. It makes sense that the driver can fiddle with the parameters if necessary, but should it know whether a parameter's length was explicitly set from the beginning?

  

 "select e.col + @param from ...

would also not work if the value of the parameter is longer than the mapped size of col."

True, but that was what I was fixing in the StartWith code, which performs a concatenation with "%".

/G