Proper support for IN clause for composite values in Criteria

[bahusoid]

Ported latest version of hibernate InExpression. As NHibernate version generates completely incorrect SQL for composite keys.

[hazzik]

@bahusoid I think you do not gain much from substituting "bogus" parameters because you have more string manipulations. I think it would be clearer (and probably more efficient) to split this code into 3 branches:

1. One column: super simple col in (@...) statement.
2. Multiple columns:
   2a. When dialect supports multi-column IN statement.
   2b. When dialect does not support multi-column IN statement.

[bahusoid]

I think you do not gain much from substituting "bogus" parameters

I'm not trying to gain anything. I need to split SQL template generation and parameters generation. This
is due to NHibernate specific parameter backtrack functionality so you have to provide parameters with typed value specified right away sqlString.Add(criteriaQuery.NewQueryParameter(typedValue) .

This bogus parameter allows me to separate template generation and parameters generation routines.

[bahusoid]

I think it would be clearer (and probably more efficient) to split this code into 3 branches:

I need to use functions like SqlStringHelper.Repeat and SqlStringHelper.Join to effectively generate SQL and "bogus" parameter allows it. Otherwise as I explained above I need to inject real parameters right in to generation process (so it won't be clearer for sure)

@hazzik So please clarify how suggested splitting allows me to effectively generate SQL without bogus parameter.

Current splitting (the same as in hibernate):

1. When dialect supports multi-column IN statement or it's one column;
2. When dialect does not support multi-column IN statement.

Suggested by you additional splitting really won't change much.

[hazzik]

I don't understand what's wrong with putting actual parameters into the query.

I've got following snippet:

/// the beginning of the method is skipped
	var parametersList = GetParameterTypedValues(criteria, criteriaQuery)
	    .Select(tv => criteriaQuery.NewQueryParameter(tv).ToArray())
	    .ToList();

	return GetSqlString(criteriaQuery, columns, parametersList);
}

private static SqlString GetSqlString(
	ICriteriaQuery criteriaQuery,
	SqlString[] columns,
	List<Parameter[]> parametersList)
{
	if (columns.Length == 1)
	{
		//single column: col1 in (?, ?)
		return new SqlString(
			columns[0],
			" in (",
			SqlStringHelper.Join(", ", parametersList.Select(p => p.Single())),
			")"
		);
	}

	if (criteriaQuery.Factory.Dialect.SupportsRowValueConstructorSyntaxInInList)
	{
		//multi column:  (col1, col2) in ((?, ?), (?, ?))
		return new SqlString(
			"(",
			SqlStringHelper.Join(", ", columns),
			") in ((",
			SqlStringHelper.Join("), (", parametersList.Select(p => SqlStringHelper.Join(", ", p))),
			"))"
		);
	}
	
	//((col1 = ? and col2 = ?) or (col1 = ? and col2 = ?))

	var objects = parametersList.Select(
		parameters => new SqlString(
			"(",
			SqlStringHelper.Join(
				" and ", columns.Zip(parameters, (c, p) => new SqlString(c, " = ", p))),
			")"));

	return new SqlString(
		"(",
		SqlStringHelper.Join(" or ", objects),
		")"
	);
}

First two can be collapsed together, but I didn't like it.

[bahusoid]

I don't understand what's wrong with putting actual parameters into the query

1. Main reason. I don't want to rewrite logic that's already present in hibernate. I practically copy-pasted it from hibernate converting values to SqlString ( and still would prefer to keep it that way);

2. Your logic doesn't look cleaner to me. IMHO for external eyes both implementations need the same amount of time to understand it;

3. Your current snippet doesn't work with ClassWithCompositeIdFixture.QueryOverInClause test. It generates wrong SQL in first case ((this_.string_, this_.short_, this_.date_) in ((@p0), (@p1)) and throw exception in second (that's SupportsRowValueConstructorSyntaxInInList == false).

[hazzik]

Your current snippet doesn't work with

Yes, I did not run tests. It has come from wrong-ish assumption on how GetParameterTypedValues work.

[hazzik]

I was reluctant to approve it because there were code changes which I think could be improved. I tried to do that, but never got time to finish.