Dapper: OrWhere() causing unexpected behavior in Where()

Created on 28 Nov 2016  路  4Comments  路  Source: StackExchange/Dapper

A prior issue resulted in this fix.

I guess no one bothered to check the original test case. It is still not "fixed":

var sql = new SqlBuilder();
var tpl = sql.AddTemplate("select * from foo /**where**/");

sql.Where("MyField = @Param");
sql.OrWhere("MyField2 = @Param2");

@rafakwolf expected:

select * from foo WHERE  ( MyField = @Param OR MyField2 = @Param2 )

However both the previous and fixed SqlBuilder produces the following:

select * from foo WHERE MyField = @Param AND  ( MyField2 = @Param2 )

This is because only the first call to one of Where() or OrWhere() is the one that defines the joiner variable. That is, the variable that dictates what string to place between clauses. If Where() is called first the joiner will be "AND" but if OrWhere() is called first the joiner will be "OR".

Consider this: with the "fix" we get the following behavior (imagine these are in separate functions that don't know about each other):

sql.Where("a = @a");

sql.OrWhere("b = @b1");
sql.OrWhere("b = @b2");

result:

select * from foo WHERE a = @a AND (b = @b1 OR b = @b2)

But switch the order of the calls:

sql.OrWhere("b = @b1");
sql.OrWhere("b = @b2");

sql.Where("a = @a");

result:

select * from foo WHERE a = @a OR (b = @b1 OR b = @b2)

And that is a new bug introduced by the change. IMO under no circumstance should .Where() result in a clause getting OR'd into the query.

As a side comment, with the previous code it was at least consistent in that it always produced the first result irrespective of order. I believe the code change was made in haste and that the original behavior was correct, albeit confusing.

Most helpful comment

I think the problem is more that it doesn't make sense to mix ANDs and ORs with a single definition.

sql.Where("a = @a1");
sql.OrWhere("b = @b1");
sql.Where("a = @a2");
sql.OrWhere("b = @b2");

should generate:

a = @a1 OR b = @b1 AND a = @a2 OR b = @b2

which isn't really a common case.

A more common case would be:

(a OR b) AND (c OR d)

How would you do that with the existing solution?

The final solution should be able to generate any arbitrary nested conditions. Also a NOT operator might be useful. Then the syntax might become unreadable so it might be an idea to use a LINQ-style Where clause with C# operators...

All 4 comments

Sorry I haven't gotten to this. I agree the old change was not the correct fix, and I don't plan on releasing the library to NuGet again until we fix this properly (possibly in a major version). I'll try and find time to revert that change for safety this weekend.

Can you open an issue for how this should behave, for all cases? There's certainly a big community using this far more than we are - let's figure out what it should be, even if that change requires a major version as a breaking change. Let's just get it right rather than trying to fix one symptom. That could definitely include extending SqlBuilder for other common cases people are hitting.

You guys and gals are keeping on top of this far more than Marc and I have time to give at the moment. We very much appreciate all the help we can get.

It seems to me that you should not be able to call OrWhere on something that is not the result of calling Where.
Where can return a more specialized type which provides the OrWhere method either directly or via an extension method.
Consider the pattern followed by LINQ in the OrderBy and ThenBy methods.

I think the problem is more that it doesn't make sense to mix ANDs and ORs with a single definition.

sql.Where("a = @a1");
sql.OrWhere("b = @b1");
sql.Where("a = @a2");
sql.OrWhere("b = @b2");

should generate:

a = @a1 OR b = @b1 AND a = @a2 OR b = @b2

which isn't really a common case.

A more common case would be:

(a OR b) AND (c OR d)

How would you do that with the existing solution?

The final solution should be able to generate any arbitrary nested conditions. Also a NOT operator might be useful. Then the syntax might become unreadable so it might be an idea to use a LINQ-style Where clause with C# operators...

Picking up what 27068 commented

Is there currently any way to build two separate OR groups with Where and OrWhere?
By separate OR groups I mean this:

(a OR b) AND (c OR d)

Many thanks

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rafakwolf picture rafakwolf  路  5Comments

unipeg picture unipeg  路  3Comments

ishamfazal picture ishamfazal  路  5Comments

valinoment picture valinoment  路  4Comments

yozawiratama picture yozawiratama  路  5Comments