Dec 21, 2012 at 10:15 PM
Edited Dec 25, 2012 at 5:22 PM
|
There is an open WorkItem about HierarchyId in the issue tracker:
http://entityframework.codeplex.com/workitem/128
I needed this feature too, so I implemeted it in my own fork:
http://entityframework.codeplex.com/SourceControl/network/forks/zgabi/EntityFrameworkHierarchyId (comment: HierarchyId support added)
I would appreciate if this modification was merged into the main branch with a contribution.
|
|
Developer
Dec 21, 2012 at 10:25 PM
|
Hi zgabi,
We would welcome a contribution in this area. Before we can look at your code you will need to submit a Contributor License Agreement (CLA) as described here:
http://entityframework.codeplex.com/wikipage?title=Contributing
We will also need to review the design and API and work with you if there need to be any changes. It would be very useful if you could give a brief description of what you have added in terms of public API and how you expect it to be used. (This may be obvious
from the code but I haven't looked at it yet due to the need for a CLA.)
Once we reach agreement on the design, API, tests, etc., then the normal process would be fore you to submit a pull request, which is also described in the link above.
I look forward to working with you on this!
Thanks,
Arthur
|
|
Dec 21, 2012 at 11:16 PM
Edited Dec 21, 2012 at 11:27 PM
|
Hi Arthur,
Tomorrow I'll print/sign/scan and send the CLA.
I have added a new public class (HierarchyId), which is almost the same as the SqlHierarchyId class, but I didnt want to add refenrece to the
Microsoft.SqlServer.Types assembly. (But tt is very easy to add a reference to that assembly and use the original SqlHierarchyId class instead of HierarchyId class)
The implementation of the HierarchyId type is very similar to the implementation of the DbGeometry and DbGeography types.
Example:
Code first model:
public class Table1
{
public int Id { get; set; }
public HierarchyId Path { get; set; }
}
public class MyContext : DbContext
{
public DbSet<Table1> Table1 { get; set; }
}
How to use:
var items = c.Table1.OrderByDescending(o => o.Path).ToArray();
foreach (var table1 in items)
{
Console.WriteLine(table1.Id + " " + table1.Path);
}
output:
4 /2/ 3 /1/2/ 2 /1/1/ 1 /1/
var items = c.Table1.Where(o=>o.Path.IsDescendantOf(new HierarchyId("/1/")) == 1).Select(
o => new
{
Id = o.Id,
OrigPath = o.Path,
Path = o.Path.GetReparentedValue(new HierarchyId("/1/"), HierarchyId.GetRoot()),
Level = o.Path.GetLevel()
}).ToArray();
foreach (var table1 in items)
{
Console.WriteLine(table1.Id + "; " + table1.OrigPath + "; " + table1.Path + "; " + table1.Level);
}
output:
1; /1/; /; 1 2; /1/1/; /1/; 2 3; /1/2/; /2/; 2
Thanks,
Gábor
|
|
Developer
Dec 21, 2012 at 11:22 PM
|
Hi Gábor,
Thanks for providing this information. Several important members of the team are on vacation at the moment. We will certainly discuss this in more detail as a priority after the holiday period and get back to you then. Having the signed CLA will make things
much easier.
Thanks again,
Arthur
|
|
|
|
Hi Arthur,
Ok, I'm on vacation, too. I checked the CLA and I can submit it only in January because I have to sign it with my employer. However I made this modification in my free time, and the commited codes are not related to my official work.
Gábor
|
|
Developer
Dec 23, 2012 at 10:57 PM
|
Just a thought - if HierarchyId is a new EDM type (I did not look at the code), wouldn't it require revving CSDL (and as a result all other artifacts)?
|
|
|
|
I tried to use hierarchyid in CDSL, and it was working. I haven't modified the CSDL parsing.
I have created a changeset (temporary added to SpatialTest class) , could you please check it (without CLA)? It contains modifications only in FunctionalTest project.
http://entityframework.codeplex.com/SourceControl/network/forks/zgabi/EntityFrameworkHierarchyId/changeset/e88ede671f9c
When I run the DbQuery_SelectMany_with_TVFs_and_spatial_types_works function the result is:
1 SRID=4326;POINT (-122.31946 47.625112) Supplier1 /1/
10 SRID=4326;POINT (-122.341529 47.611693) Supplier10 /3/
11 SRID=4326;POINT (-122.352842 47.6186) Supplier11 /3/1/
...
Both the insert (in SpatialNorthwindInitializer.cs) and the select works.
|
|
Developer
Dec 24, 2012 at 6:08 PM
|
I will wait for the CLA before I take a look.
|
|
Developer
Jan 11 at 11:03 PM
|
zgabi,
Thanks for submitting the CLA. One of the EF team members is going to look at your code and also do some broader thinking around the best way to support hierarchy IDs and will get back to you.
Thanks,
Arthur
|
|
Developer
Jan 23 at 5:48 PM
|
Hi zgabi,
We have spent some time on the EF team brainstorming what we think the best approach is for tackling hierarchy ID and other related features. The conclusions from these discussions are as follows:
- It is currently hard for EF to support new data types because the Entity Data Model (EDM) needs to be updated to support each new type. There are times when adding types as first-class members of EDM adds enough value that it is worth doing all the work
and reving the EDM version. However, there are also cases where the value of this is not high, and we believe that HierarchyID is probably one of these cases. In such cases we really want to make it easy to support new types without the EF internals having
hard-coded information about those types. Instead we would need some form of extension points that can handle reading and converting from a data reader and also converting appropriately for updates. There probably also needs to be a way for the store model
to handle the new type so that areas like Migrations can handle scaffolding for them.
- The dependency we took on the SQL Types assembly for spatial is problematic in several ways—for example, different versions of the assembly, needing to download and install the assembly on the client machine even when the rest of EF is bin deployable, the
dynamic binding, and so on. It may be that we need to use the SQL Types assembly for HierarchyID but it is not clear that this is the case.
- It’s not clear what the most useful mappings are for hierarchy ID. Having a type like the one you added may be an approach we take, but it couples the application’s model assembly to EF, which is usually something we avoid. (I know that the spatial types
do this as well, but this is a compromise we wish we could have avoided.) In line with the first bullet point, it would be good if the type we map to is not fixed. It may also be useful to map things like navigation properties over the hierarchical data, and
in such cases maybe the HierarchyID itself can just be stored as a string property. We don’t necessarily need all of this to be done in a contribution, but whatever mapping we do choose should be flexible enough to handle such things in the future.
- The schedule and work we have planned for EF6 doesn’t allow us the time to go deeply into the design for everything above. However, we do plan to return to it after EF6 is released. At that time we would like to re-engage with you if you are still interested
and potentially take some of your code or any additional work you do in this area as a contribution.
In summary, we appreciate greatly the work you have done here and your desire to contribute to EF, but for now we’re going to hold off on the contribution until we have a bit more time to ensure that we’re creating the best underlying building blocks for
this area.
Thanks,
Arthur
|
|