Add support for custom result mapping

[moozzyk]

Ported from: https://codefirstfunctions.codeplex.com/workitem/6

REPORTED ON:
REPORTED BY:
Jul 21, 2014 at 9:26 PM
moozzyk

In EF 6.1.1 it is possible to define custom mappings between columns returned by a store function and properties of entity/complex type the result of store function is mapped to (https://entityframework.codeplex.com/workitem/2192). The convention currently allows only 1:1 mapping (i.e. the name of the column has to match the name of the property). Consider adding support for custom result mapping where the user can specify which column will be mapped to which property.

moozzyk wrote Jul 22, 2014 at 11:27 PM [x]

As per Angel's comment to the post - further changes to EF codebase are required to support custom result mapping:

_Yes, those commits were a prerequisite for custom mappings, thanks for authoring them.

The code missing is for NonComposable functions with ResultMappings, that creates a new EdmItemCollection when StorageMappingItemCollection is null.

Look at the changes you made in file Core/Mapping/FunctionImportMappingNonComposable.cs in commit b4cb2b1. The consturctor of FunctionImportMappingNonComposable crashes if resultMappings.Any() is true.

I’m currently using the code from my fork in production and it works great with custom mappings, but we do not use non composable functions.

[danielloganking]

I would like to say explicitly that this is something that I would like to see brought into master. We have been using @angelyordanov's fork in production for awhile now and it works exceptionally well for composable functions. I understand the concern with breakage in the FunctionImportMappingNonComposable ctor when resultMappings.Any() is not null. The following:

resultMapping => new FunctionImportStructuralTypeMappingKB(
    resultMapping.TypeMappings,
    containerMapping.StorageMappingItemCollection.EdmItemCollection))`

is gonna blow up in your face. Even still I think the code is solid enough for addition, if only because it has been rock solid in our production API for over a year and half.

There seems to be some recent discussion around this that I do not understand however (mostly because it is cryptic and the user never really followed up). @bazarco seems to be seeking a different direction, likely for supporting the API/design of the EntityFramework.Functions project he has forked. I bring this all up as a way of saying a PR to add a similar

var edmItemCollection =
    _entityContainerMapping.StorageMappingItemCollection != null
    ? _entityContainerMapping.StorageMappingItemCollection.EdmItemCollection
    : new EdmItemCollection(new EdmModel(DataSpace.CSpace));

to the else block of FunctionImportMappingNonComposable may meet with resistance. @angelyordanov's work then might reasonably merged to the master with a single alteration. Where he has

model.ConceptualToStoreMapping.AddFunctionImportMapping(
    new FunctionImportMappingNonComposable(
        functionImportDefinition,
        storeFunctionDefinition,
        resultMappings.Any() ? resultMappings.ToArray() : new FunctionImportResultMapping[0],
        model.ConceptualToStoreMapping));

in FunctionConvention.cs simply don't pass resultMappings.ToArray and instead always pass new FunctionImportResultMapping[0]. Then note in a hack comment the functionality depends on a PR for EF itself and until then will only support non-composable functions via conventional naming. It's an inelegant solution but worth consideration for the additional of non-conventional mapping.

And lastly, thanks for this library. The amount of headache it has prevented for me is exceptional and I sincerely believe we could not have the performance or scale we do without this functionality. Just thinking about the implications for our stack if this doesn't exist is huge. Thank you.

[moozzyk]

Related dotnet/ef6#75

dotnet/ef6@b4cb2b1?diff=unified#commitcomment-19230430

[moozzyk]

I need to spend some time trying to understand the problem - it's been almost three years since I looked at this. If I do find time I will try to submit a PR in EF6.

[danielloganking]

I'm working on merging Angel's code locally with the latest version of master and running into an odd situation. Using the master branch code and copying Angel's work into the FunctionConvention.cs I get an error from EF specifying that "The Column 'customName' specified as part of this MSL does not exist in MetadataWorkspace." No such error occurs when the code is placed in the old "beta" branch where the original non-conventional naming work was done. It's all very strange. I'm trying to track it down now but there may be some mismatch between the work done to move out of beta2 and into the full release and Angel's work. Alternatively there could be some deeper cause like changes made between EF 6.1.0 and 6.1.1 (the respective versions referenced by Code First Functions beta and latest) Nothing is jumping out at me yet but I'm going to keep hacking away until I find the root of the problem.

[danielloganking]

I have solved the issue and am working on a pull request. Need to clean up some of the end to end tests but beyond that everything is now behaving as expected. I did not stray from @angelyordanov's code much (if at all) since that has been what we've proven to work in production. The bulk of the credit should go to him. I expect the pull request will be submitted sometime early next week.

[moozzyk]

As per my comment to @angelyordanov pull request in codeplex my understanding is that the changes broke an existing feature. If this is the case I am not going to merge this change - it may work for you because you are not using the feature it breaks but someone may be already be using it and if they upgraded the package they would be broken if though they don't care about the feature you are introducing.

[danielloganking]

Do you remember which features it broke? I'd be comfortable rewriting it to not introduce a regression but I don't know what to look for outside of the test suite. I would look at the old branch but I think @angelyordanov has taken it down. At least, I can no longer find it on GitHub and am working off my local copy.

[angelyordanov]

Hi @DanielLKing and @moozzyk.

Its been long ago and I can't quite remember all the details but I think this code will not work for NonComposable functions. This was the required change in the EF code to make it work.
https://entityframework.codeplex.com/SourceControl/network/forks/angelyordanov/entityframework/changeset/4f7c8d53275d1a5c43f2508a546dab029b47b3b0#src/EntityFramework/Core/Mapping/FunctionImportMappingNonComposable.cs

@DanielLKing are the tests passing?

[danielloganking]

All but 2 tests pass without me doing anything. I haven't tried to resolve the last two tests because I was waiting to hear which regressions were potentially introduced and make sure that I didn't fix the code for the last 2 tests only to have to start over again. The 2 tests though seemed to be a setup issue and not a real regression because of the setup changing due to adding a test for custom column name.

[moozzyk]

@DanielLKing @angelyordanov - Above I pointed to this comment: https://codefirstfunctions.codeplex.com/SourceControl/network/forks/angelyordanov/codefirstfunctions/contribution/7271. It mentions that the change broke sprocs returning collections of scalar values. All the features should have tests so if a feature is broken tests should catch it.

Regarding the missing branch I moved this project to github merely a couple weeks - take a look at the repo on codeplex - the branch is probably still there.