views:

65

answers:

3

As I'm struggling to learn LINQ I’ve managed to generate a SQL statement with "AND (0 = 1)" as part of the where clause. I'm just wondering if this result is common in poorly written queries and is a known issues to try and avoid or if I am doing something totally backwards to end up with this.


Update

public static IEnumerable<ticket> GetTickets(stDataContext db,string subgroup, bool? active)
    {
        var results = from p in db.tickets
               where 
                   ( active == null || p.active == active ) 
                   /*(active == null ? true :
                   ((bool)active ? p.active : !p.active))*/ &&
                   p.sub_unit == db.sub_units.Where(c=>subgroup.Contains(c.sub_unit_name))
               select p;
        return results;
    }

If I ignore the active part and just run

public static IEnumerable<ticket> GetTickets1(stDataContext db,string subgroup, bool? active)
    {
        return db.tickets.Where(c => c.sub_unit.sub_unit_name == subgroup);
    }

It returns the groups of tickets I want ignoring the active part.

+1  A: 

I'd pull the processing out of the ternary operators.

where ( active == null || p.active == active )

EDIT

The rest of the where clause looks funky too... why is it not just doing

&& p.sub_unit.sub_unit_name == subgroup

or

&& subgroup.Contains(p.sub_unit.sub_unit_name)

?

Tanzelax
Both of those changes fixed the query results, thanks.
Tim
A: 

This is probably caused by a null value in one you the columns you have declared as non-nullable. LINQ2SQL makes all columns non-nullable by default. Go back to the designer and change the fields to allow values to be null. Unfortunately this feature is By Design (see connect.microsoft.com link below.)

(linq) incorrect sql generated for row.column == localVar when localVar is null (should be "is null" check)

Matthew Whited
+1  A: 

That is some pretty heavy abuse of the ternary operator.

This expression:

(active == null ? true :
    ((bool)active ? p.active : !p.active))

Is equivalent to the following logic:

bool result;
if (active == null)
{
    result = true;
}
else
{
    if ((bool)active)
    {
        result = p.active;
    }
    else
    {
        result = !p.active;
    }
}
result &= ...

Think carefully about what this is doing:

  • If active is null, you're fine, it skips to the next condition.
  • If active is true, result is true at the end of the conditional.
  • If active is false, result is false at the end of the conditional.

In the last case, the query can never return any rows!

@Tanzelax has already supplied a simple rewrite. The main idea is that you want to compare p.active to active, not actually evaluate the condition as p.active.

Aaronaught
Yeah this is open source code, so I didn't write that line, or clearly look at it close enough. I've been chuckling since Tanzelax posted that. I'll update the above code to reflect the change.
Tim