Rebind base parameter in PredicateBuilder instead of using Invoke

[EdwardBlair]

This is untested in this project.. in so far as all I did was change the code and commit it to see if it passes unit tests. It's based on a custom stripped down ExpressionBuilder I wrote earlier this evening.

Unless I have missed something, is there a need to use Expression.Invoke(...) here? All chained expressions will - should - have the same parameter shouldn't they?

Using the RebindParameterVisitor we should then be able to use the PredicateBuilder's predicates within Select, SelectMany and GroupBy selectors.

This, at least, works fine for me. However all I'm using this for is to dynamically filter out certain entities from my context. So there may be some more advanced use cases where this breaks everything!

Something like this contrived e.g.

    IQueryable<Purchase> GetPurchases()
    {
        var customerPredicate = PredicateBuilder.New<Customer>(true);
        var productPredicate = PredicateBuilder.New<Product>(true);

        if (true /* Some Statement */)
        {
            // Demo criteria to filter customers
            var customerIds = new int[] {1, 2};
            customerPredicate = customerPredicate.And(customer => customerIds.Contains(customer.Id));

            var fiveDaysAgo = DateTime.Now.AddDays(-5);
            purchasePredicate = purchasePredicate.And(purchase => purchase.Date >= fiveDaysAgo);
        }

        return dataContext.Customers.Where(customerPredicate).SelectMany(customer => customer.Purchases.AsQueryable().Where(purchasePredicate));
    }

Feedback please!

[EdwardBlair]

Unsure why appveyor hasn't appended this PR with a tick, but it did pass all tests.

[EdwardBlair]

Fixed bug in commit 8b10932

[EdwardBlair]

This causes a stack overflow if we do something like

fooPredicate.And(foo => foo.Bars.Any(bar => bar.Baz == "qux"));

Should be just return base.VisitParameter(node);

Unsure why it crashes, probably because we never Accept when the Parameters differ in type, but interesting nonetheless.

[fsateler]

There are conflicts on this PR. @EdwardBlair could you fix that please?

I believe this is the correct approach, as replacing the parameters is much simpler than invoking and then bypassing the invoke via the query expander.

[EdwardBlair]

Thanks, updated 😄

[StefH]

Did you also add some unit-tests for this specific issue?

[EdwardBlair]

For 8b10932 ?

No, I haven't. That's because I was calling the wrong base method in my overridden one -- a typo, not really something that should go wrong otherwise.

I can't do if you'd like!

All existing PredicateBuilder tests run (ran -- build is broken in your branch) fine 👍

[StefH]

New NuGet uploaded to https://www.myget.org/F/linqkit/api/v3/index.json

Please test, and if this is ok, I can publish to NuGet.org

[StefH]

@EdwardBlair Did you have time to test this?

[fsateler]

I downloaded the package from myget and tested it in linqpad. Works as expected:

Account.Where(PredicateBuilder.New<Account>().Or(acc => acc.Id < 1).Or(acc => acc.FirmId > 4))

This throws with 1.1.8 but works with 1.1.9.

[StefH]

OK. I will publish to NuGet.