Validate that PaginateViaPrimaryKey is not used without Take

[AlekseyMartynov]

Inspired by #372

Possible options:

• throw without Take
• auto-disable without Take

[coeamyd]

Reposted here from #336 discussion:

I would say, since pagination requires a Take parameter, PaginateViaPrimaryKey should not be used, if the take parameter is not specified. Without it, we are simply NOT paging, so any pagination options should not have any effect. I would prefer a warning, but that will be a bit more diffcult to implement :-)

I vote for silently disabling PaginateViaPrimaryKey, since we are not paging in this case.

On the other hand, if take is very large, we could also run into the same issue. The maximum allowed parameters for SqlServer are 2.100, afaik. So, if we do paging via PK, we would need Take * PrimaryKey.Length parameters to perform it. If we see that this value is > 2.100, we should probably also either throw an exception or disable it silently. I think, an exception here would be more appropriate, so the developer gets feedback, that he is doing something wrong.

So, maybe we need a differentiated approach: If we are not paging (i.e. no Take), I would disable it silently. If we are paging with large values, we should notify the developer, that the option will not work.

P.S.: 2.100 is probably a bad threshold, since the underlying query may also need additional parameters. We should leave some headroom there. On the other hand: Paging with a page size of > 1000 is kind of absurd in the first place :-)

[coeamyd]

To work around the issue, maybe you could enhance the client and server side code to include the actually used page size. Then, you could limit the page size on the server to a feasible amount when using PaginateViaPrimaryKey. When the client side detects, that the actual page size returned by the server is less than the requested amount, it can issue further requests, until all data has been loaded. This would make it transparent for developers, but means some more effort. The question is: Is it worth it?

[AlekseyMartynov]

@coeamyd

For the first take, I'm adding the Take > 0 condition. Pardon the pun.

Upper bound checks are an interesting topic. This can be useful for other load options too. For example, to limit group/sort level, etc. We'll keep it for possible future enhancements.

Thank you.

[coeamyd]

Sounds good to me. AFAIK, Microsoft's OData v4 implementation also enables you to specify upper bounds for other options, so, yeah, seems like a good candidate for a future enhancement.