Support JSON columns in compiled models #30254
Conversation
| var jsonColumnName = entityType.GetContainerColumnName()!; | ||
| var jsonColumnTypeMapping = (entityType.GetViewOrTableMappings().SingleOrDefault()?.Table | ||
| ?? entityType.GetDefaultMappings().Single().Table) | ||
| .FindColumn(jsonColumnName)!.StoreTypeMapping; |
There was a problem hiding this comment.
@AndriySvyryd I'm not sure if this is right. Should it just look on the DefaultMappings? Does the column need to be added to both the table mappings and the default mappings? If so, what's the appropriate way to do this? Should it be the same column instance?
There was a problem hiding this comment.
Default mapping is currently used for FromSql, so yes, the column should be added to the default mapping also
There was a problem hiding this comment.
Here you should decide whether to use Table, View or default mapping based on what mapping is being used for the container entity type
| var jsonColumnTypeMapping = targetEntityType.GetContainerColumnTypeMapping()!; | ||
| var jsonColumnTypeMapping = (entityType.GetViewOrTableMappings().SingleOrDefault()?.Table | ||
| ?? entityType.GetDefaultMappings().Single().Table) | ||
| .FindColumn(jsonColumnName)!.StoreTypeMapping; |
There was a problem hiding this comment.
Same question here.
|
@maumar @AndriySvyryd As I understand it, query won't use the default mapping unless there is no table or view mapping. Do we have existing query tests for the default mapping? (Presumably, FromSql tests where the entity type is explicitly not mapped to a table or view?) If so, where? |
ajcvickers
force-pushed
the
JsonAndTheCompilonauts029
branch
from
9166ddc to
4b56e9b
Compare
|
@AndriySvyryd @maumar Updated to store the container column in the default mapping. |
|
| @@ -97,7 +99,7 @@ public virtual Type ProviderClrType | |||
| /// doing so can result in application failures when updating to a new Entity Framework Core release. | |||
| /// </summary> | |||
| public virtual RelationalTypeMapping StoreTypeMapping | |||
| => PropertyMappings.First().TypeMapping; | |||
| => _storeTypeMapping ?? PropertyMappings.First().TypeMapping; | |||
There was a problem hiding this comment.
Store the result in _storeTypeMapping
Fixes #29602 The main change here is to store the JSON type mapping in the relational model, and obsolete the previous storage in the underlying EF model.
ajcvickers
force-pushed
the
JsonAndTheCompilonauts029
branch
from
4b56e9b to
f73a22e
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #29602
The main change here is to store the JSON type mapping in the relational model, and obsolete the previous storage in the underlying EF model.