Issue with multiple "Take" methods.

Posts   
 
    
arash
User
Posts: 54
Joined: 16-Dec-2008
# Posted on: 13-Oct-2013 13:02:58   

Hi,

I'm facing an issue with LLBLGen 3.5 when using following binaries in Adapter mode:

  • SD.LLBLGen.Pro.DQE.SqlServer.NET20.dll (3.5.12.317)

  • SD.LLBLGen.Pro.LinqSupportClasses.NET35.dll (3.5.13.429)

  • SD.LLBLGen.Pro.ORMSupportClasses.NET20.dll (3.5.13.725)

  • SD.LLBLGen.Pro.QuerySpec.dll (3.5.12.710)

The problem is that when I apply two "Take" extension methods on a data source the first one is ignored in generated SQL command. For example the following query will return 10 records (which I expect to return 5 records):

var q = metadata.Product.Take(5).Take(10);
var list = q.ToList();

I know that applying "Take()" like this seems ridiculous, but It happens for me when I use ASP.Net Web Api OData provider. As you may know, OData services have the capability of applying default page size. For example if the Products table contains millions of records and the user sends a request without any criteria or page size:

http://TheServer/OData/Products

The server only returns a predefined amount of data to the client and the client is responsible to send another request for the next chunk of data.

The issue raises when user applies a top parameter in the request, and the server page size for example is set to 10000:

http://TheServer/OData/Products?$top=5

The service returns 10000 records. I tracked down the issue and it seems the generated expression tree (generated by Microsoft) contains two "Take" methods. I checked it with Entity Framework and it's working fine. Two nested select query will be generated with each corresponding top parameter. But in LLBL runtime just one select statement is generated and the other one is ignored.

Thanks.

daelmo avatar
daelmo
Support Team
Posts: 8245
Joined: 28-Nov-2005
# Posted on: 14-Oct-2013 07:30:58   

Are you using the LLBLGen OData support classes?

David Elizondo | LLBLGen Support Team
arash
User
Posts: 54
Joined: 16-Dec-2008
# Posted on: 14-Oct-2013 07:39:30   

No, I don't use it. I'm just using simple linq queries.

arash
User
Posts: 54
Joined: 16-Dec-2008
# Posted on: 14-Oct-2013 08:24:00   

Actually I'm using Asp.Net Web Api OData Provider, but this problem can be reproduced using a simple linq query.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39912
Joined: 17-Aug-2003
# Posted on: 14-Oct-2013 11:07:46   

A second Take call overwrites any previous Take method's value, that's true. We don't generate a subquery with an inner Take wrapped with the second Take. It is indeed a limitation, coming from the fact that not all databases support a limit statement inside a subquery and it always requires an orderby clause as well, which isn't always given, plus if distinct violating types are in the projection, the TOP clause will return wrong rows and this can't be circumvented in a subquery. Entity framework doesn't check on this and will return a page of duplicates, we don't. The downside is that we can't do a Take method in a subquery. It's rare that one needs one and an outer query as well though.

This might sound silly, but I don't understand this:

The server only returns a predefined amount of data to the client and the client is responsible to send another request for the next chunk of data.

The issue raises when user applies a top parameter in the request, and the server page size for example is set to 10000:

Why do you set the first take to 10000 and not to a small amount? Or is this a paging system other than take+skip?

Frans Bouma | Lead developer LLBLGen Pro
arash
User
Posts: 54
Joined: 16-Dec-2008
# Posted on: 14-Oct-2013 16:15:05   

Why do you set the first take to 10000 and not to a small amount? Or is this a paging system other than take+skip?

Actually this is another paging system. Microsoft's OData implementations (WCF Data Services & Web Api OData) have a default page size service parameter which prevents the clients to send a request for a large chunk of data. You may find a WCF Data services sample here:

http://msdn.microsoft.com/en-us/library/ee473424.aspx

After setting the page size, if the user sends a request without any skip + take or sending it with a large number the server would only sends small amount of data to the client.This way the client is avoided from intentionally or accidentally requesting large amount of data and consuming server resources intensively.

The 10000 was a sample, in our case we are actually using 500 limit. So if the client sends a request without any skip + take or sends a request for more than 500 records, the server will send only 500 records plus a link referring to the next page of data(It's an odata standard). If the client uses paging explicity, for example sends a request with $skip being set to 10 and $top being set to 20, the server will return just 20 records. The problem is that both pagings are applied, first the Take(20) and then the Take(500) and as you mentioned the Take(20) is ignored in LLBL.

This is happening for me on Asp.net Web Api OData provider, I'm not sure about WCF Data Services. It seems that the ASP.NET team assumes that applying multiple paging would be fine as they have surely tested it against their own Entity Framework or Linq to Objects. I would appreciate if this is fixable on LLBL side. If not, I will take a look at ASP.NET source code on codeplex. In my experience, the MVC and Web Api frameworks are written extensible and hopefully I'll find a way to extend and customize the paging mechanism.

Thank you in Advance.

Walaa avatar
Walaa
Support Team
Posts: 14995
Joined: 21-Aug-2005
# Posted on: 14-Oct-2013 23:09:13   

IMHO you should either enable the user to do the paging or set a default paging size at the server side.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39912
Joined: 17-Aug-2003
# Posted on: 15-Oct-2013 09:29:18   

@Walaa, that would not have the safety stop of the max limit the method will ever return (so whatever the user specifies, the method will never return N rows, e.g. 500). WIth a public service I can imagine one doesn't want to rely on clients to do 'the right thing', if a wrong query could bring the server down because it fetches all rows.

@arash, I've been thinking about this, and I have to clarify a bit: if a Take() call is specified on a sequence in a Linq query which in itself is seen as a separate query (so not merged with an outer query/sequence), the inner Take is kept as-is, as it should. However multiple Take() calls on effectively the same query are not: the most outer Take call will overwrite the inner one: in the simplest situation (sequence.Take(n).Take(m)), it will simply overwrite the value on the handled 'sequence' expression for the # of elements to fetch, in the situation where you have sequence.Take(n).SomeOtherMethod().SomeOtherMethod2()....Take(m), and everything can be merged together into 1 query (e.g. linqMetaData.Customer.Take(n).OrderBy(..).Take(m)), we also don't keep the inner query, but merge them all together for efficiency reasons, but this will also overwrite the inner one with the outer one.

In both situations it's rather silly to do so, we concluded, because linqMetaData.Customer.Take(10).Take(20), will never result in 20 rows because the set to select 20 on has at most 10 rows. The same is true for: linqMetaData.Customer.Take(20).Take(10), it will never result in 20 rows, as the Take(10) will make sure at most 10 rows will returned.

This means that if a value for MaxNumberOfElementsToReturn is already set to a value, and it is about to be overwritten with value V, it only should do so if V is smaller than the existing value (and >=0) otherwise it should ignore the value.

This is rather easy to add, problem is: it's a breaking change: if we make this change, it will be changing resultsets for queries which rely on this. So we'll add a setting for this (static) which is undocumented for now, which will be present in v3.5, 4.0 and further, and which will by default result in the current behavior, but when switched off, it will be the behavior you need: When multiple Take calls are present in the same query, the smaller value wins.

In v4.1 (currently in development) we'll reverse this setting's default value, meaning we'll introduce the breaking change but people who want to keep the old behavior can do so by using the (then documented) switch. We'll make this change because we concluded it's a bug in our code: the behavior our code currently has is silly, and therefore we introduce this change back in v3.5.

I'll post in this thread again with an updated v3.5 runtime and how to use it so you'll get the queries you need.

Frans Bouma | Lead developer LLBLGen Pro
arash
User
Posts: 54
Joined: 16-Dec-2008
# Posted on: 15-Oct-2013 10:10:46   

This means that if a value for MaxNumberOfElementsToReturn is already set to a value, and it is about to be overwritten with value V, it only should do so if V is smaller than the existing value (and >=0) otherwise it should ignore the value.

I totally agree with you.I believe the ASP.NET team has to to change the way they apply paging as it is unnecessary to wrap it with an extra select statement, they should first choose the correct page size and then apply it to the query sequence.

Thanks for your great support.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39912
Joined: 17-Aug-2003
# Posted on: 15-Oct-2013 10:19:40   

simple_smile

Ok, got it working. See attached dll. By default, you'll see nothing has changed, you actively have to set a setting to false to make it work, as in the example below. It's a static setting and it's recommended for you to do this once, at the beginning of your service instance, e.g. in Application_Start or something like that.


[Test]
public void MultipleTakeCallsTest()
{
    using(var adapter = new DataAccessAdapter())
    {
        var metaData = new LinqMetaData(adapter);
        var q = metaData.Customer.Take(10).Take(20);

        try
        {
            LLBLGenProProviderBase.AlwaysUseLastSpecifiedTake = false;
            Assert.AreEqual(10, q.ToList().Count);
        }
        finally
        {
            LLBLGenProProviderBase.AlwaysUseLastSpecifiedTake = true;
        }
    }
}

Above, I set the switch to false prior to fetching the query. I reset it afterwards for unit testing, but you should keep it to false once set. LLBLGenProProviderBase is a class in the LinqSupportClasses assembly.

Hope this fixes your issue, if not, let me know. simple_smile

Attachments
Filename File size Added on Approval
SD.LLBLGen.Pro.LinqSupportClasses.NET35.dll 245,760 15-Oct-2013 10:19.49 Approved
Frans Bouma | Lead developer LLBLGen Pro
arash
User
Posts: 54
Joined: 16-Dec-2008
# Posted on: 15-Oct-2013 10:25:34   

I'll check it. Thankssimple_smile

arash
User
Posts: 54
Joined: 16-Dec-2008
# Posted on: 15-Oct-2013 14:33:47   

It's working fine. Thanks.