Linq where clause with predicate fails

[Johan Nilsson]

Sorry for the somewhat unclear subject. Here's an NUnit repro where the second test case fails (Count == 2 instead of 1):

---

class WithId

{

  public string Id { get; set; }

}

public class RavenQueryTests

{

  private IDocumentStore store;

  private string id1;

  private string id2;

  [SetUp]

  public void InitFixture()

  {

    store = new EmbeddableDocumentStore {RunInMemory = true}.Initialize();

 

    using (var session = store.OpenSession())

    {

      var o1 = new WithId();

      var o2 = new WithId();

      session.Store(o1);

      session.Store(o2);

      session.SaveChanges();

      id1 = o1.Id;

      id2 = o2.Id;

    }

  }

  [Test]

  public void Query()

  {

    Assert.That(

      store.OpenSession().Query<WithId>().Where(_ => _.Id == id1).ToList(),

      Has.Count.EqualTo(1)

    );  

  }

  [Test]

  public void QueryWithPredicate()

  {

    Predicate<WithId> idEqualToId1 = _ => _.Id == id1;

    Assert.That(

      store.OpenSession().Query<WithId>().Where(_ => idEqualToId1(_)).ToList(),

      Has.Count.EqualTo(1)

    );

  }

}

---

Using RavenDB Embedded 1.0.531.

[Matt Warren]

You need to change "idEqualToId1" in your 2nd query to this:

    Expression<Predicate<WithId>> idEqualToId2 = _ => _.Id == "id1";

I think something is getting messed up in the query due to the way you're supplying the predicate. In your 1st test the compiler sorts it all out for you.

Also you need to use WaitForNonStaleResultsAsOfNow(..) in unit tests otherwise your results could be inconsistent.

[Matt Warren]

But of a copy-and-paste error, the code should be this:

[Johan Nilsson]

On Tuesday, December 6, 2011 4:34:44 PM UTC+1, Matt Warren wrote:

You need to change "idEqualToId1" in your 2nd query to this:

    Expression<Predicate<WithId>> idEqualToId2 = _ => _.Id == "id1";

Simply changing the line to the above leaves me with a compile error from the "Where" clause (I fixed the typos above):

error CS0118: 'idEqualToId1' is a 'variable' but is used like a 'method'

Which makes sense, but I don't really know how to fix it. I can compile the expression and call through that instead:

      Assert.That(

        store.OpenSession().Query<WithId>().Where(_ => idEqualToId1.Compile()(_)).ToList(),

        Has.Count.EqualTo(1)

      );

But that ends up with the same problem (all items are returned).

/ Johan

[Oren Eini (Ayende Rahien)]

This should work

        store.OpenSession().Query<WithId>().Where( idEqualToId1).ToList(),

[Johan Nilsson]

Den onsdagen den 7:e december 2011 kl. 09:29:17 UTC+1 skrev Ayende Rahien:

This should work

        store.OpenSession().Query<WithId>().Where( idEqualToId1).ToList(),

 

The following test:

    [Test]

    public void QueryWithExpressionPredicate()

    {

      Expression<Predicate<WithId>> idEqualToId1 = _ => _.Id == id1;

      Assert.That(

        store.OpenSession().Query<WithId>().Where(idEqualToId1).ToList(),

        Has.Count.EqualTo(1)

      );

    }

Fails to compile with:

Server.Repository.Raven\Server.Repository.Raven.Tests\Characterization\RavenQueryByPredicateTests.cs(65,9): error CS1928: 'Raven.Client.Linq.IRavenQueryable<Server.Repository.Raven.Tests.Characterization.WithId>' does not contain a definition for 'Where' and the best extension method overload 'Raven.Client.Linq.LinqExtensions.Where<T>(Raven.Client.Linq.IRavenQueryable<T>, System.Linq.Expressions.Expression<System.Func<T,bool>>)' has some invalid arguments

Server.Repository.Raven\Server.Repository.Raven.Tests\Characterization\RavenQueryByPredicateTests.cs(65,51): error CS1503: Argument 2: cannot convert from 'System.Linq.Expressions.Expression<System.Predicate<Server.Repository.Raven.Tests.Characterization.WithId>>' to 'System.Linq.Expressions.Expression<System.Func<Server.Repository.Raven.Tests.Characterization.WithId,bool>>'

Changing the type of idEqualToId1 to "Expression<Func<WithId, bool>>" as the error hints seems to fix the problem though. As where clauses always returns a bool there should perhaps be an overload for this?

/ Johan

[Oren Eini (Ayende Rahien)]

Johan,

What are you _trying to do_? 

[Johan Nilsson]

Den onsdagen den 7:e december 2011 kl. 10:50:59 UTC+1 skrev Ayende Rahien:

Johan,

What are you _trying to do_? 

I was just about to post that as the given solution doesn't workin my context.

I've got an existing repository interface like:

interface IRepository<T>

{

  ...

  IQueryable<T> Matches(Predicate<T> predicate);

  ...

}

That I'm trying to implement using an embedded RavenDB backend.

[Matt Warren]

Changing the type of idEqualToId1 to "Expression<Func<WithId, bool>>" as the error hints seems to fix the problem though. As where clauses always returns a bool there should perhaps be an overload for this?

Of course, I missed that bit, your predicate need to return a bool to indicate a match, otherwise it's not really doing anything.

Expression<Func<WithId, bool>> idEqualToId2 = _ => _.Id == "id1"; 

[Oren Eini (Ayende Rahien)]

No, don't do things like that.

First, that interface is going to drastically reduce the usability that you can get from the client API.

Second, you are trying to pass a delegate, but Linq requires expressions. Moreover, the linq API requires Func<...>, to work. 

That is outside our own purview.

[Johan Nilsson]

A predicate returns a bool per definition (it's implicit).

[Oren Eini (Ayende Rahien)]

That isn't how Linq defines most of its ops, however.

[Johan Nilsson]

On Wednesday, December 7, 2011 11:07:08 AM UTC+1, Ayende Rahien wrote:

No, don't do things like that.

Ok.

 

First, that interface is going to drastically reduce the usability that you can get from the client API.

The interface is supposed to be pretty narrow (and it's already defined).

 

Second, you are trying to pass a delegate, but Linq requires expressions.

Ok. I'm far from an advanced Linq user which explains my problems I guess. Still, IMHO, if Linq requires expressions it should not accept delegates and then produce unexpected behavior at runtime.

 

Moreover, the linq API requires Func<...>, to work. 

I'm not sure I follow you. This test passes:

    [Test]

    public void QueryWithPredicateOnArray()

    {

      WithId[] ids = {new WithId {Id = id1}, new WithId {Id = id2}};

      Predicate<WithId> idEqualToId1 = _ => _.Id == id1;

      Assert.That(

        ids.Where(_ => idEqualToId1(_)).ToList(),

        Has.Count.EqualTo(1)

      );

    }

/ Johan

[Oren Eini (Ayende Rahien)]

You are running Linq to Objects, with everything compiled.

[Johan Nilsson]

Ok. I guess that one could say that the different Linq implementations breaks LSP. Not much to do about that perhaps.

[Matt Warren]

Ok. I guess that one could say that the different Linq implementations breaks LSP. Not much to do about that perhaps.

 

I suppose, except that have to do different things.

LINQ-to-SQL, LINQ-to-EF or the RavenDB LINQ provider need an Expression<Func<..>> so that they can parse the expression tree and generate the relevant SQL or RavenDB server calls.

Whereas LINQ-to-Objects is all in-memory so it only needs a Func<..> that it can execute when needed.

Normally the compiler hides all this from you and does the work necessary depending on whether your source is IEnumerable<T> or IQueryable<T>. But in your case you're not doing it all in-line, instead you're storing the function in a variable, so you have to do the Expression<Func<..>> bit manually,

[Johan Nilsson]

Hi,

On Thursday, December 8, 2011 10:36:58 AM UTC+1, Matt Warren wrote:

Ok. I guess that one could say that the different Linq implementations breaks LSP. Not much to do about that perhaps.

 

I suppose, except that have to do different things.

I realize that. I'm not complaining about RavenDB specifically; just Linq in general.

 

LINQ-to-SQL, LINQ-to-EF or the RavenDB LINQ provider need an Expression<Func<..>> so that they can parse the expression tree and generate the relevant SQL or RavenDB server calls.

Whereas LINQ-to-Objects is all in-memory so it only needs a Func<..> that it can execute when needed.

Normally the compiler hides all this from you and does the work necessary depending on whether your source is IEnumerable<T> or IQueryable<T>. But in your case you're not doing it all in-line, instead you're storing the function in a variable, so you have to do the Expression<Func<..>> bit manually,

Yes. But being obnoxious, my expectation would still be for the code to behave the same independent of the provider - that's why I referenced LSP (or the Principle of Least Surprise, or ...). At least I'd like to get a compile error, or, at the very least have an NotSupportedException thrown as soon as possible instead of silently getting invalid results back.

But that's just my 0.02EUR.

Regards / Johan

[Oren Eini (Ayende Rahien)]

Johan,

Linq is anything but a provider neutral mechanism. It is a language integrated query, but it doesn't abstract the queries or how they work