jaydata icon indicating copy to clipboard operation
jaydata copied to clipboard

single() should not send $top clause if filter is against a primary key

Open oising opened this issue 12 years ago • 13 comments

If I call:

context.MyEntities.single(function(e) { return e.ID == 1 })

then Jaydata calls an oData service with:

/service?$filter=ID eq 1&$top=2

I know the $top=2 is an optimization to test if single() is violated (i.e. count > 1) but it clashes with another optimization that Microsoft's OData toolkit does: If a where clause contains only a primary key, then $top is invalid (since primary keys are guaranteed unique - 0 or 1 count) and the query fails.

May I suggest that jaydata omit the top clause if the filter is solely against a primary key.

oising avatar Jan 03 '13 20:01 oising

Yes, sure. It'll be in 1.2.7 coming this week. Thanks.

PeterAronZentai avatar Jan 07 '13 11:01 PeterAronZentai

Actually in the case of addressing a single entry, why not use the natural OData syntax? i.e.

/api/MyEntitySet(5)

or for multi-property key:

/api/MyEntitySet(FirstKey=5,SecondKey=3)

It certainly is more natural and easier to debug.

khaledh avatar Feb 03 '13 20:02 khaledh

Becouse theb youd lose the power of jayData

mohammadchehab avatar Feb 03 '13 20:02 mohammadchehab

I guess I wasn't clear. I meant that JayData itself should use the proper OData syntax I mentioned, not the develoepr himself/herself.

I was actually surprised that the method $data.EntitySet.find(keyValue, cb) internally uses the single method (which in turn is translated to a $filter query) instead of properly addressing the single resource by primary key as I mentioned above.

khaledh avatar Feb 03 '13 20:02 khaledh

single() is not about retrieving data by primary key. It's a generic function where you can define a filter expression which should only return one data, less or more than one is an error. So you can say: cars.single('it.color == 'Red' and it.Manufacturer == 'Ferrari') A special case of this generic filter when you filter by the primary key. Yes, it could be handled in a special way so maybe we will, but I can not promise it. ItemStore api already has a function which could do this but it doesn't.

agdolla avatar Feb 03 '13 20:02 agdolla

I agree with your argument about single(). But I find that the semantics of find() requires the use of direct addressing of the resource using the primary key, and not through the generic single() method.

khaledh avatar Feb 03 '13 20:02 khaledh

Where did you find about the 'find()' function ? Just curious.... In our tutorials we usually have filter, first, single, I can not remember seeing find() lately. Right now I'd say do not use find().

agdolla avatar Feb 03 '13 21:02 agdolla

It's in the source code. You're right in that it's not mentioned in the docs. I guess I was surprised that there was no direct way to get a specific entity by primary key, so I looked at the source code and I found find().

I guess I have this expectation that every data access framework should provide a method to get a specific entity by PK. I use JayData with an OData-enabled ASP.NET Web API (still in preview), which sits on top of EF code first. So the chain of commands in this case as I see them would be:

  1. [JayData] context.Products.find(1)
  2. [OData] /api/Products(1)
  3. [Web API] ProductsController.GetEntityByKey(1)
  4. [EF] dbContext.Products.Find(1)

You see, the bottom three layers of the stack all provide a method to get an entity by key. JayData does as well (although not documented), but its implementation doesn't play nicely with the rest of the stack. What JayData currently does is the following:

  1. [JayData] context.Products.single(function(product) { return product.ProductID == 1 }, {}, ...)
  2. [OData] /api/Products?$filter=(ProductID eq 1)&top=2
  3. [Web API] ProductsController.Get()
  4. [EF] dbContext.Products
  5. [Web API QueryableAttribute] IQueryable.Where( p => p.ProductID == 1).Take(2)

I can live with single() for now, but it would be good if JayData exposes find() eventually with proper addressing of entities by PK.

khaledh avatar Feb 04 '13 00:02 khaledh

Do you have an sql server which serves EF ? dbContext.Products.Find(1) and IQueryable.Where( p => p.ProductID == 1) will eventually generate the same sql statement (select from Products where ProductID = 1) because sql does not have a separate command to retrieve data by primary key, The sql server will parse the sql command and figure out that it can use a certain index. So while I agree that we could have a method to retrieve data by primary key, the current solution is just as good performance wise. find() method is not supported and please do not use it, we might remove it at any time and without any note which will break your code. Use only supported and documented features, please.

agdolla avatar Feb 04 '13 08:02 agdolla

I understand that both will eventually generate the same SQL, so the concern wasn't about performance as much as it was about conceptual integrity of the different layers.

As I mentioned earlier, I'm currently using single() for this purpose, but was hoping that JayData would eventually provide a "GetByKey/GetById/Find" kind of method. Thanks.

khaledh avatar Feb 05 '13 16:02 khaledh

Sorry, this is a miss understanding. Find is there for that purpose. please notice that Find does not support composite keys yet however

Provider specific ways of getting a solo item by key should indeed be in the library on a better way, some efforts have already been put to it. I just created an issue #82 for this regarding OData.

PeterAronZentai avatar Feb 05 '13 16:02 PeterAronZentai

Also note that by mixing in some Item Store API methods (out of the box there), you can access an entity with it's key by simply writing

EntityType.read(keyValue).then( function(item) {console.log(item); })

To have an entity type know its service location etc it must

  • a) be inferred with $data.service("service.uri", cb)
  • b) store must be introduced with EntityType.setStore( storeName, storeDefinition)

storeDefinition example: { provider: 'oData', dataSource: 'my.svc/products'}

This approach has already some optimizations implemented, for the time being only for the WebAPI provider: EntityType.read() actuall invokes GET /entityController/id and does not touch the query subsystem.

PeterAronZentai avatar Feb 05 '13 16:02 PeterAronZentai

Hi Peter that last comment was very helpful, just made the code a lot easier to work with for getting single entities.. i didn't see that read method in the documentation. Must have missed it or something.

The raw request sent a filter &top 2 which works out for me...

I used the jaysvcutil to generate the entity types from my odata metadata so it all worked out

ghost avatar Apr 01 '13 14:04 ghost