Fix for #3500
Please see https://github.com/nhibernate/nhibernate-core/issues/3500 for issue
Just heads up for reviewers: I cannot reproduce the issue, so I believe this fix is not necessary/not correct.
The unit test from hazzik (checking SqlLogSpy) is different from my unit test, I am checking the command.text sent to the SQL server,
This issue definitely caused my code to have the problem which has now gone away. I've checked the other unit tests and they are still passing, so this fix does no harm.
I've run through the test that @hazzik had, and the result is different from mine, but also what they are testing for. My test is using the SqlDriver, and the hazzik unit test is using SQlLogSpy. I may have uncovered a new bug, that wasn't picked up in the previous tests.
Please can it be reviewed.
Before my fix command.commandText SELECT * FROM Test where value = @p0 or value=@p1
SQLLogSpy: NHibernate.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)]
After my fix command.commandText SELECT * FROM Test where value = @p0 or value=@p0
SqlLogSpy: 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)]
This issue definitely caused my code to have the problem which has now gone away.
You would need to provide a test that shows the real issue, not a synthetic test that tests for what you've just done (these tests are worthless). Synthetic test can help when you have identified the issue and want to isolate the fix. But you have not yet succeeded in doing this.
You need to demonstrate your issue top to bottom: from a mapping to a query.
I've checked the other unit tests and they are still passing, so this fix does no harm.
This is nonconsecutive. There are unknowns unknowns that might break.
@hazzik After some further investigation, I found a castle core interceptor that was firing, this was doing something to the NHibernate.SqlCommand.SqlString which was upsetting the parameter processing.
I am going to cancel this PR, thanks for questioning the PR, and making me find the proper source of the problem 👍
Glad you've found the issue @tskong