nhibernate-core icon indicating copy to clipboard operation
nhibernate-core copied to clipboard

RemoveUnusedCommandParameters() removing needed parameters

Open tskong opened this issue 1 year ago • 9 comments

Hi,

I'm running this linq statement, but it's erroring as not all the SQL parameters are being presented to SQL server, I have been debugging through the nhibernate source and the culprit is partially RemoveUnusedCommandParameters(), but I could do with some help in diagnosing it.

The C# code uses listContactGroupId twice within the linq statement, and that seems to confuse things.

listContactGroupId = new List<int>() { 7, 14, 21, 28 };
var res = dbObject.Count(x => (x.EnquiryContactGroupId.HasValue && 
listContactGroupId.Contains(x.EnquiryContactGroupId.Value)) || (x.ApplicationContactGroupId.HasValue && listContactGroupId.Contains(x.ApplicationContactGroupId.Value)));

The SQL Looks like this

select (count(*)) as col_0_0_ from EnquiryApplicationContactDetail_V venquiryap0_ where (venquiryap0_.EnquiryContactGroupId is not null) and (venquiryap0_.EnquiryContactGroupId in (@p0 , @p1 , @p2 , @p3)) or (venquiryap0_.ApplicationContactGroupId is not null) and (venquiryap0_.ApplicationContactGroupId in (@p4 , @p5 , @p6 , @p7))

But when presented to the SQL server, parameter values @p4->@P7 are missing.

The parameters go missing at the method RemoveUnusedCommandParameters(), there is a line that removes the duplicates

var assignedParameterNames = new HashSet<string>(formatter.AssignedParameterNames); The formatter.AssignedParmetersNames, rather than being @p0, @p1, @p2, @p3,@P4, @p5, @p6, @P7 it's @p0, @p1, @p2, @p3, @p0, @p1, @p2, @p3, so it ends up with @p0, @p1, @p2, @p3

It's got something to do with nhibernate.sqlcommand.parameter.ParameterPosition which assigns the @p[n] parameter name.

Can anyone shed any light on how it generates the parameter names?

Thanks,

tskong avatar Mar 08 '24 14:03 tskong

I have a bit more information. It appears that "backtrack" on the parameters has something to do with it, 0-3 and 4-7 have the same backtrack id, so it think 4-7 are the same parameters and then it discards them, even though they are needed. What is this backtrack for, and can I inhibit it?

image

tskong avatar Mar 09 '24 21:03 tskong

it think 4-7 are the same parameters and then it discards them

yes, they are same and this is how it meant to be and how it should work.

even though they are needed

they are not.

You’re, unfortunately, looking at this from a wrong angle. You need to find why these parameters got duplicated and why SQL generated new parameters instead of reusing the existing.

The SQL generated should be this one:

select (count(*)) as col_0_0_ from EnquiryApplicationContactDetail_V venquiryap0_ where (venquiryap0_.EnquiryContactGroupId is not null) and (venquiryap0_.EnquiryContactGroupId in (@p0 , @p1 , @p2 , @p3)) or (venquiryap0_.ApplicationContactGroupId is not null) and (venquiryap0_.ApplicationContactGroupId in (@p0 , @p1 , @p2 , @p3))

hazzik avatar Mar 09 '24 23:03 hazzik

Thanks for responding.

Yes, you're right, that's exactly what I want it to do, but I need some ideas as to how it works out the parameters, I've not worked on nhibernate source before and it's taking me a while to understand how it all fits together

The interesting thing is that if I make a reference to the list, then @p0->@p7 and it's values are submitted, so whatever it's doing it knows the references are different.

var listContactGroupId = new List<int>() { 7, 14, 21, 28 };
var listCopy = listContactGroupId 
var res = dbObject.Count(x => (x.EnquiryContactGroupId.HasValue && 
listContactGroupId.Contains(x.EnquiryContactGroupId.Value)) || (x.ApplicationContactGroupId.HasValue && listCopy .Contains(x.ApplicationContactGroupId.Value)));

In my original use case, it knows that the parameters @p0-@p3 are the same as @p4-@p7 so it discards @p4-@p7 from the supplied parameter values, but it keeps those parameters in the query, causing sql to error as the values for @p4-P7 are not supllied.

tskong avatar Mar 09 '24 23:03 tskong

I've come up with a fix now, and wanted to run it past everyone before I create a PR, I've ran it on our code base and it works fine.

The fix I've done is to run through the parameters, and instead of deleting a parameter, finding it's equivalent via the parameter position and replacing it with that. e.g.

@p0 parameterposition 0

@p1 parameterposition 1

@p3 (actually the same as @p0 as it's the same entity) parameterposition 0

@p4 (actually the same as @p1 as it's the same entity) parameterposition 1

DriverBase.cs

	public void RemoveUnusedCommandParameters(DbCommand cmd, SqlString sqlString)
	{
		if (!UseNamedPrefixInSql)
			return; // Applicable only to named parameters

		var formatter = GetSqlStringFormatter();
		formatter.Format(sqlString);
		// the parms @p0-P7 are now @p0-p3 twice, so the duplicates are removed.
		var assignedParameterNames = new HashSet<string>(formatter.AssignedParameterNames);

               
		if (assignedParameterNames.Count != formatter.AssignedParameterNames.Count())
		{
			// Some parameters should be removed
			int index = 0;
			// Iterate from the parameters and replace those which are aliased.
			foreach (DbParameter p in cmd.Parameters)
			{
				if (!assignedParameterNames.Contains(UseNamedPrefixInParameter ? p.ParameterName : FormatNameForSql(p.ParameterName)))
				{
					// instead of just removing, replace with the parameter it's substituted with
					Parameter parameter = sqlString.GetParameters().ElementAt(index);
					if (parameter?.ParameterPosition != null)
					{
						// rename the parameter name with it's replacement
						var replacementParameterName = UseNamedPrefixInSql
							? NamedPrefix + ToParameterName(parameter.ParameterPosition.Value)
							: ToParameterName(parameter.ParameterPosition.Value);
						cmd.CommandText = cmd.CommandText.Replace(p.ParameterName, replacementParameterName);
					}
				}
				index++;
			}

			// Delete any parameters that aren't used.
			cmd.Parameters
				.Cast<DbParameter>()
				.Select(p => p.ParameterName)
				.Where(p => !assignedParameterNames.Contains(UseNamedPrefixInParameter ? p : FormatNameForSql(p)))
				.ToList()
				.ForEach(unusedParameterName => cmd.Parameters.RemoveAt(unusedParameterName));
		}
	}

tskong avatar Mar 11 '24 16:03 tskong

Hi @tskong I could not reproduce your original issue. This is the SQL I'm getting from fairly similar query:

linq:

List<int> x = [7, 14, 21, 28];

var result = session
             .Query<Entity>()
             .Count(
	             e => e.GroupId1.HasValue && x.Contains(e.GroupId1.Value) ||
	                  e.GroupId2.HasValue && x.Contains(e.GroupId2.Value));

sql:

    select
        (count(*)) as col_0_0_ 
    from
        Entity entity0_ 
    where
        (
            entity0_.GroupId1 is not null
        ) 
        and (
            entity0_.GroupId1 in (
                @p0 , @p1 , @p2 , @p3
            )
        ) 
        or (
            entity0_.GroupId2 is not null
        ) 
        and (
            entity0_.GroupId2 in (
                @p0 , @p1 , @p2 , @p3
            )
        );
    @p0 = 7 [Type: Int32 (0:0:0)], @p1 = 14 [Type: Int32 (0:0:0)], @p2 = 21 [Type: Int32 (0:0:0)], @p3 = 28 [Type: Int32 (0:0:0)]

What version of NHibernate do you use? What DB? What dialect and driver?

hazzik avatar Apr 04 '24 15:04 hazzik

Hi,

That's very strange, your code is pretty much the same as mine.

I'm using fluent nhibernate so I don't know if that will affect it, as I found the fix within the nhibernate code.

nhibernate is v5.2.0.0, it's the SqlClientDriver, MsSql2008Dialect.

fluent nhibernate is 2.1.2.0

tskong avatar Apr 04 '24 16:04 tskong

Any interceptors or custom code? I've added test here: https://github.com/hazzik/nhibernate-core/commit/44f8c61d9690008da0738eba1a7bf0998a569260 and they pass on all of the platforms we test with.

hazzik avatar Apr 04 '24 22:04 hazzik

@tskong that's too old. Could you please check on more recent version of NH?

hazzik avatar Apr 05 '24 01:04 hazzik

Your unit test looks correct. when I reproduced this, I brought in the latest nhibernate

<NhVersion Condition="'$(NhVersion)' == ''" >5.6</NhVersion>

And I had the same error, and the fix I did, made it go away. I was using the SQL profiler, so that's where I saw the extra parameters I didn't need.

My unit test is different from yours though, I am checking the command.CommandText, this would be the command sent to the SQL server.

tskong avatar Apr 05 '24 15:04 tskong

As stated by #3520, the trouble was external.

fredericDelaporte avatar Apr 28 '24 20:04 fredericDelaporte