views:

1525

answers:

4

I ran into a scenario where LINQ to SQL acts very strangely. I would like to know if I'm doing something wrong. But I think there is a real possibility that it's a bug.
The code pasted below isn't my real code. It is a simplified version I created for this post, using the Northwind database.
A little background: I have a method that takes an IQueryable of Product and a "filter object" (which I will describe in a minute). It should run some "Where" extension methods on the IQuerable, based on the "filter object", and then return the IQueryable.
The so-called "filter object" is a System.Collections.Generic.List of an anonymous type of this structure: { column = fieldEnum, id = int }
the fieldEnum is an enum of the different columns of the Products table that I would possibly like to use for the filtering.
Instead of explaining further how my code works, it's easier if you just take a look at it. It's simple to follow.

enum filterType { supplier = 1, category }
public IQueryable<Product> getIQueryableProducts()
{
    NorthwindDataClassesDataContext db = new NorthwindDataClassesDataContext();
    IQueryable<Product> query = db.Products.AsQueryable();

    //this section is just for the example. It creates a Generic List of an Anonymous Type
    //with two objects. In real life I get the same kind of collection, but it isn't hard coded like here
    var filter1 = new { column = filterType.supplier, id = 7 };
    var filter2 = new { column = filterType.category, id = 3 };
    var filterList = (new[] { filter1 }).ToList();
    filterList.Add(filter2);

    foreach(var oFilter in filterList)
    {
        switch (oFilter.column)
        {
         case filterType.supplier:
                query = query.Where(p => p.SupplierID == oFilter.id);
                break;
            case filterType.category:
                query = query.Where(p => p.CategoryID == oFilter.id);
                break;
            default:
                break;
        }
    }
    return query;
}

So here is an example. Let's say the List contains two items of this anonymous type.
The first being { column = fieldEnum.Supplier, id = 7 }
The second being { column = fieldEnum.Category, id = 3}.
After running the code above, the underlying SQL query of the IQueryable object should be
...WHERE SupplierID = 7 AND CategoryID = 3
But in reality, after the code runs the SQL that gets executed is
...WHERE SupplierID = 3 AND CategoryID = 3
I tried defining 'query' as a property and setting a breakpoint on the setter, thinking I could catch what's changing it when it shouldn't be. But everything was supposably fine. So instead I just checked the underlying SQL after every command. I realized that the first 'Where' runs fine, and the 'query' stays fine (meaning SupplierID = 7) until right after the foreach loop runs the second time. Right after 'oFilter' becomes the second anonymous type item, and not the first, the 'query' SQL changes to 'Supplier = 3'. So what must be happening here under-the-hood is that instead of just remembering that Supplier should equal 7, LINQ to SQL remembers that Supplier should equal oFilter.id. But oFilter is a name of a single item of a foreach loop, and it means something different after it iterates.

+4  A: 

I have only glanced at your question, but I am 90% sure that you should read the first section of

On lambdas, capture, and mutability

(which includes links to 5 similar SO questions) and all will become clear.

Brian
+1  A: 

Working as designed. The issue you are confronting is the clash between lexical closure and mutable variables.

What you probably want to do is

foreach(var oFilter in filterList)
{
    var o = oFilter;
    switch (o.column)
    {
        case filterType.supplier:
            query = query.Where(p => p.SupplierID == o.id);
            break;
        case filterType.category:
            query = query.Where(p => p.CategoryID == o.id);
            break;
        default:
            break;
    }
}

When compiled to IL, the variable oFilter is declared once and used multiply. What you need is a variable declared separately for each use of that variable within a closure, which is what o is now there for.

While you're at it, get rid of that bastardized Hungarian notation :P.

Justice
"mutable variables" - isn't "o" a mutable variable? perhaps there's a better way to describe the problem.
David B
A: 

The problem is that you're not appending to the query, you're replacing it each time through the foreach statement.

You want something like the PredicateBuilder - http://www.albahari.com/nutshell/predicatebuilder.aspx

Slace
A: 

I think this is the clearest explanation I've ever seen: http://blogs.msdn.com/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx:

Basically, the problem arises because we specify that the foreach loop is a syntactic sugar for

{
    IEnumerator<int> e = ((IEnumerable<int>)values).GetEnumerator();
    try
    {
        int m; // OUTSIDE THE ACTUAL LOOP
        while(e.MoveNext())
        {
            m = (int)(int)e.Current;
            funcs.Add(()=>m);
        }
    }
    finally
    {
        if (e != null) ((IDisposable)e).Dispose();
    }
}

If we specified that the expansion was

try
{
    while(e.MoveNext())
    {
        int m; // INSIDE
        m = (int)(int)e.Current;
        funcs.Add(()=>m);
    }

then the code would behave as expected.

Dinah