From b270581467bc6a309845af5a6ea7f6008f3db0c1 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Thu, 25 Aug 2022 13:46:21 -0700 Subject: [PATCH] Query: Change JSONQuery.Path to be list of PathSegment Resolves #28838 Resolves #28836 --- .../Query/EntityProjectionExpression.cs | 43 +- .../Query/JsonQueryExpression.cs | 456 +++++++++--------- src/EFCore.Relational/Query/PathSegment.cs | 49 ++ .../SqlExpressions/JsonScalarExpression.cs | 176 +++---- .../Query/SqlExpressions/SelectExpression.cs | 42 +- ...rchConditionConvertingExpressionVisitor.cs | 10 +- .../Internal/SqlServerQuerySqlGenerator.cs | 17 +- 7 files changed, 372 insertions(+), 421 deletions(-) create mode 100644 src/EFCore.Relational/Query/PathSegment.cs diff --git a/src/EFCore.Relational/Query/EntityProjectionExpression.cs b/src/EFCore.Relational/Query/EntityProjectionExpression.cs index 6d54e0b473b..a1454869810 100644 --- a/src/EFCore.Relational/Query/EntityProjectionExpression.cs +++ b/src/EFCore.Relational/Query/EntityProjectionExpression.cs @@ -22,7 +22,7 @@ public class EntityProjectionExpression : Expression /// /// Creates a new instance of the class. /// - /// The entity type to shape. + /// An entity type to shape. /// A dictionary of column expressions corresponding to properties of the entity type. /// A to generate discriminator for each concrete entity type in hierarchy. public EntityProjectionExpression( @@ -115,7 +115,6 @@ public virtual EntityProjectionExpression MakeNullable() discriminatorExpression = ce.MakeNullable(); } - var primaryKeyProperties = GetMappedKeyProperties(EntityType.FindPrimaryKey()!); var ownedNavigationMap = new Dictionary(); foreach (var (navigation, shaper) in _ownedNavigationMap) { @@ -123,18 +122,8 @@ public virtual EntityProjectionExpression MakeNullable() { // even if shaper is nullable, we need to make sure key property map contains nullable keys, // if json entity itself is optional, the shaper would be null, but the PK of the owner entity would be non-nullable intially - Debug.Assert(primaryKeyProperties != null, "Json entity type can't be keyless"); - var jsonQueryExpression = (JsonQueryExpression)shaper.ValueBufferExpression; - var ownedPrimaryKeyProperties = GetMappedKeyProperties(shaper.EntityType.FindPrimaryKey()!)!; - var nullableKeyPropertyMap = new Dictionary(); - for (var i = 0; i < primaryKeyProperties.Count; i++) - { - nullableKeyPropertyMap[ownedPrimaryKeyProperties[i]] = propertyExpressionMap[primaryKeyProperties[i]]; - } - - // reuse key columns from owner (that we just made nullable), so that the references are the same - var newJsonQueryExpression = jsonQueryExpression.MakeNullable(nullableKeyPropertyMap); + var newJsonQueryExpression = jsonQueryExpression.MakeNullable(); var newShaper = shaper.Update(newJsonQueryExpression).MakeNullable(); ownedNavigationMap[navigation] = newShaper; } @@ -145,34 +134,6 @@ public virtual EntityProjectionExpression MakeNullable() propertyExpressionMap, ownedNavigationMap, discriminatorExpression); - - static IReadOnlyList? GetMappedKeyProperties(IKey? key) - { - if (key == null) - { - return null; - } - - if (!key.DeclaringEntityType.IsMappedToJson()) - { - return key.Properties; - } - - // TODO: fix this once we enable json entity being owned by another owned non-json entity (issue #28441) - - // for json collections we need to filter out the ordinal key as it's not mapped to any column - // there could be multiple of these in deeply nested structures, - // so we traverse to the outermost owner to see how many mapped keys there are - var currentEntity = key.DeclaringEntityType; - while (currentEntity.IsMappedToJson()) - { - currentEntity = currentEntity.FindOwnership()!.PrincipalEntityType; - } - - var count = currentEntity.FindPrimaryKey()!.Properties.Count; - - return key.Properties.Take(count).ToList(); - } } /// diff --git a/src/EFCore.Relational/Query/JsonQueryExpression.cs b/src/EFCore.Relational/Query/JsonQueryExpression.cs index 491ce382206..ee742a7a8c4 100644 --- a/src/EFCore.Relational/Query/JsonQueryExpression.cs +++ b/src/EFCore.Relational/Query/JsonQueryExpression.cs @@ -3,275 +3,253 @@ using Microsoft.EntityFrameworkCore.Query.SqlExpressions; -namespace Microsoft.EntityFrameworkCore.Query +namespace Microsoft.EntityFrameworkCore.Query; + +/// +/// +/// An expression representing an entity or a collection of entities mapped to a JSON column and the path to access it. +/// +/// +/// This type is typically used by database providers (and other extensions). It is generally +/// not used in application code. +/// +/// +public class JsonQueryExpression : Expression, IPrintableExpression { + private readonly IReadOnlyDictionary _keyPropertyMap; + + /// + /// Creates a new instance of the class. + /// + /// An entity type being represented by this expression. + /// A column containing JSON value. + /// A map of key properties and columns they map to in the database. + /// A type of the element represented by this expression. + /// A value indicating whether this expression represents a collection or not. + public JsonQueryExpression( + IEntityType entityType, + ColumnExpression jsonColumn, + IReadOnlyDictionary keyPropertyMap, + Type type, + bool collection) + : this( + entityType, + jsonColumn, + keyPropertyMap, + path: new List { new("$") }, + type, + collection, + jsonColumn.IsNullable) + { + } + + private JsonQueryExpression( + IEntityType entityType, + ColumnExpression jsonColumn, + IReadOnlyDictionary keyPropertyMap, + IReadOnlyList path, + Type type, + bool collection, + bool nullable) + { + Check.DebugAssert(entityType.FindPrimaryKey() != null, "primary key is null."); + + EntityType = entityType; + JsonColumn = jsonColumn; + IsCollection = collection; + _keyPropertyMap = keyPropertyMap; + Type = type; + Path = path; + IsNullable = nullable; + } + + /// + /// The entity type being represented by this expression. + /// + public virtual IEntityType EntityType { get; } + + /// + /// The column containg JSON value. + /// + public virtual ColumnExpression JsonColumn { get; } + + /// + /// The value indicating whether this expression represents a collection. + /// + public virtual bool IsCollection { get; } + + /// + /// The list of path segments leading to the entity from the root of the JSON stored in the column. + /// + public virtual IReadOnlyList Path { get; } + + /// + /// The value indicating whether this expression is nullable. + /// + public virtual bool IsNullable { get; } + + /// + public override ExpressionType NodeType => ExpressionType.Extension; + + /// + public override Type Type { get; } + /// - /// Expression representing an entity or a collection of entities mapped to a JSON column and the path to access it. + /// Binds a property with this JSON query expression to get the SQL representation. /// - public class JsonQueryExpression : Expression, IPrintableExpression + public virtual SqlExpression BindProperty(IProperty property) { - private readonly IReadOnlyDictionary _keyPropertyMap; - private readonly bool _nullable; - - /// - /// Creates a new instance of the class. - /// - /// An entity type being represented by this expression. - /// A column containing JSON. - /// A value indicating whether this expression represents a collection. - /// A map of key properties and columns they map to in the database. - /// A type of the element represented by this expression. - public JsonQueryExpression( - IEntityType entityType, - ColumnExpression jsonColumn, - bool collection, - IReadOnlyDictionary keyPropertyMap, - Type type) - : this( - entityType, - jsonColumn, - collection, - keyPropertyMap, - type, - path: new SqlConstantExpression(Constant("$"), typeMapping: null), - jsonColumn.IsNullable) + if (!EntityType.IsAssignableFrom(property.DeclaringEntityType) + && !property.DeclaringEntityType.IsAssignableFrom(EntityType)) { + throw new InvalidOperationException( + RelationalStrings.UnableToBindMemberToEntityProjection("property", property.Name, EntityType.DisplayName())); } - private JsonQueryExpression( - IEntityType entityType, - ColumnExpression jsonColumn, - bool collection, - IReadOnlyDictionary keyPropertyMap, - Type type, - SqlExpression path, - bool nullable) + if (_keyPropertyMap.TryGetValue(property, out var match)) { - Check.DebugAssert(entityType.FindPrimaryKey() != null, "primary key is null."); - - EntityType = entityType; - JsonColumn = jsonColumn; - IsCollection = collection; - _keyPropertyMap = keyPropertyMap; - Type = type; - Path = path; - _nullable = nullable; + return match; } - /// - /// The entity type being projected out. - /// - public virtual IEntityType EntityType { get; } - - /// - /// The column containg JSON value on which the path is applied. - /// - public virtual ColumnExpression JsonColumn { get; } - - /// - /// The value indicating whether this expression represents a collection. - /// - public virtual bool IsCollection { get; } - - /// - /// The JSON path leading to the entity from the root of the JSON stored in the column. - /// - public virtual SqlExpression Path { get; } - - /// - /// The value indicating whether this expression is nullable. - /// - public virtual bool IsNullable => _nullable; - - /// - public override ExpressionType NodeType => ExpressionType.Extension; - - /// - public override Type Type { get; } - - /// - /// Binds a property with this JSON query expression to get the SQL representation. - /// - public virtual SqlExpression BindProperty(IProperty property) - { - if (!EntityType.IsAssignableFrom(property.DeclaringEntityType) - && !property.DeclaringEntityType.IsAssignableFrom(EntityType)) - { - throw new InvalidOperationException( - RelationalStrings.UnableToBindMemberToEntityProjection("property", property.Name, EntityType.DisplayName())); - } + var newPath = Path.ToList(); + newPath.Add(new PathSegment(property.GetJsonPropertyName()!)); - if (_keyPropertyMap.TryGetValue(property, out var match)) - { - return match; - } + return new JsonScalarExpression( + JsonColumn, + property, + newPath, + IsNullable || property.IsNullable); + } - var pathSegment = new SqlConstantExpression( - Constant(property.GetJsonPropertyName()), - typeMapping: null); - - var newPath = new SqlBinaryExpression( - ExpressionType.Add, - Path, - pathSegment, - typeof(string), - typeMapping: null); - - return new JsonScalarExpression( - JsonColumn, - property, - newPath, - _nullable || property.IsNullable); + /// + /// Binds a navigation with this JSON query expression to get the SQL representation. + /// + /// The navigation to bind. + /// An JSON query expression for the target entity type of the navigation. + public virtual JsonQueryExpression BindNavigation(INavigation navigation) + { + if (navigation.ForeignKey.DependentToPrincipal == navigation) + { + // issue #28645 + throw new InvalidOperationException( + RelationalStrings.JsonCantNavigateToParentEntity( + navigation.ForeignKey.DeclaringEntityType.DisplayName(), + navigation.ForeignKey.PrincipalEntityType.DisplayName(), + navigation.Name)); } - /// - /// Binds a navigation with this JSON query expression to get the SQL representation. - /// - /// The navigation to bind. - /// An JSON query expression for the target entity type of the navigation. - public virtual JsonQueryExpression BindNavigation(INavigation navigation) + var targetEntityType = navigation.TargetEntityType; + var newPath = Path.ToList(); + newPath.Add(new PathSegment(targetEntityType.GetJsonPropertyName()!)); + + var newKeyPropertyMap = new Dictionary(); + var targetPrimaryKeyProperties = targetEntityType.FindPrimaryKey()!.Properties.Take(_keyPropertyMap.Count); + var sourcePrimaryKeyProperties = EntityType.FindPrimaryKey()!.Properties.Take(_keyPropertyMap.Count); + foreach (var (target, source) in targetPrimaryKeyProperties.Zip(sourcePrimaryKeyProperties, (t, s) => (t, s))) { - if (navigation.ForeignKey.DependentToPrincipal == navigation) - { - // issue #28645 - throw new InvalidOperationException( - RelationalStrings.JsonCantNavigateToParentEntity( - navigation.ForeignKey.DeclaringEntityType.DisplayName(), - navigation.ForeignKey.PrincipalEntityType.DisplayName(), - navigation.Name)); - } + newKeyPropertyMap[target] = _keyPropertyMap[source]; + } - var targetEntityType = navigation.TargetEntityType; - var pathSegment = new SqlConstantExpression( - Constant(navigation.TargetEntityType.GetJsonPropertyName()), - typeMapping: null); - - var newJsonPath = new SqlBinaryExpression( - ExpressionType.Add, - Path, - pathSegment, - typeof(string), - typeMapping: null); - - var newKeyPropertyMap = new Dictionary(); - var targetPrimaryKeyProperties = targetEntityType.FindPrimaryKey()!.Properties.Take(_keyPropertyMap.Count); - var sourcePrimaryKeyProperties = EntityType.FindPrimaryKey()!.Properties.Take(_keyPropertyMap.Count); - foreach (var (target, source) in targetPrimaryKeyProperties.Zip(sourcePrimaryKeyProperties, (t, s) => (t, s))) - { - newKeyPropertyMap[target] = _keyPropertyMap[source]; - } + return new JsonQueryExpression( + targetEntityType, + JsonColumn, + newKeyPropertyMap, + newPath, + navigation.ClrType, + navigation.IsCollection, + IsNullable || !navigation.ForeignKey.IsRequiredDependent); + } - return new JsonQueryExpression( - targetEntityType, - JsonColumn, - navigation.IsCollection, - newKeyPropertyMap, - navigation.ClrType, - newJsonPath, - _nullable || !navigation.ForeignKey.IsRequiredDependent); + /// + /// Makes this JSON query expression nullable. + /// + /// A new expression which has property set to true. + public virtual JsonQueryExpression MakeNullable() + { + var keyPropertyMap = new Dictionary(); + foreach (var (property, columnExpression) in _keyPropertyMap) + { + keyPropertyMap[property] = columnExpression.MakeNullable(); } - /// - /// Makes this JSON query expression nullable. - /// - /// A new expression which has property set to true. - public virtual JsonQueryExpression MakeNullable() - { - var keyPropertyMap = new Dictionary(); - foreach (var (property, columnExpression) in _keyPropertyMap) - { - keyPropertyMap[property] = columnExpression.MakeNullable(); - } + return new JsonQueryExpression( + EntityType, + JsonColumn.MakeNullable(), + keyPropertyMap, + Path, + Type, + IsCollection, + nullable: true); + } - return MakeNullable(keyPropertyMap); - } + /// + public virtual void Print(ExpressionPrinter expressionPrinter) + { + expressionPrinter.Append("JsonQueryExpression("); + expressionPrinter.Visit(JsonColumn); + expressionPrinter.Append($", {string.Join("", Path.Select(e => e.ToString()))})"); + } - /// - /// Makes this JSON query expression nullable re-using existing nullable key properties - /// - /// A new expression which has property set to true. - internal virtual JsonQueryExpression MakeNullable(IReadOnlyDictionary nullableKeyPropertyMap) - => Update( - JsonColumn.MakeNullable(), - nullableKeyPropertyMap, - Path, - nullable: true); - - /// - public virtual void Print(ExpressionPrinter expressionPrinter) - { - expressionPrinter.Append("JsonQueryExpression("); - expressionPrinter.Visit(JsonColumn); - expressionPrinter.Append($", \"{string.Join(".", Path)}\")"); - } + /// + protected override Expression VisitChildren(ExpressionVisitor visitor) + { + var jsonColumn = (ColumnExpression)visitor.Visit(JsonColumn); - /// - protected override Expression VisitChildren(ExpressionVisitor visitor) - { - var jsonColumn = (ColumnExpression)visitor.Visit(JsonColumn); - var jsonPath = (SqlExpression)visitor.Visit(Path); + // TODO: also visit columns in the _keyPropertyMap? + return Update(jsonColumn, _keyPropertyMap); + } - // TODO: also visit columns in the _keyPropertyMap? - return Update(jsonColumn, _keyPropertyMap, jsonPath, IsNullable); + /// + /// Creates a new expression that is like this one, but using the supplied children. If all of the children are the same, it will + /// return this expression. + /// + /// The property of the result. + /// The map of key properties and columns they map to. + /// This expression if no children changed, or an expression with the updated children. + public virtual JsonQueryExpression Update( + ColumnExpression jsonColumn, + IReadOnlyDictionary keyPropertyMap) + => jsonColumn != JsonColumn + || keyPropertyMap.Count != _keyPropertyMap.Count + || keyPropertyMap.Zip(_keyPropertyMap, (n, o) => n.Value != o.Value).Any(x => x) + ? new JsonQueryExpression(EntityType, jsonColumn, keyPropertyMap, Path, Type, IsCollection, IsNullable) + : this; + + /// + public override bool Equals(object? obj) + => obj != null + && (ReferenceEquals(this, obj) + || obj is JsonQueryExpression jsonQueryExpression + && Equals(jsonQueryExpression)); + + private bool Equals(JsonQueryExpression jsonQueryExpression) + => EntityType.Equals(jsonQueryExpression.EntityType) + && JsonColumn.Equals(jsonQueryExpression.JsonColumn) + && IsCollection.Equals(jsonQueryExpression.IsCollection) + && IsNullable == jsonQueryExpression.IsNullable + && Path.SequenceEqual(jsonQueryExpression.Path) + && KeyPropertyMapEquals(jsonQueryExpression._keyPropertyMap); + + private bool KeyPropertyMapEquals(IReadOnlyDictionary other) + { + if (_keyPropertyMap.Count != other.Count) + { + return false; } - /// - /// Creates a new expression that is like this one, but using the supplied children. If all of the children are the same, it will - /// return this expression. - /// - /// The property of the result. - /// The map of key properties and columns they map to. - /// The property of the result. - /// The property of the result. - /// This expression if no children changed, or an expression with the updated children. - public virtual JsonQueryExpression Update( - ColumnExpression jsonColumn, - IReadOnlyDictionary keyPropertyMap, - SqlExpression path, - bool nullable) - => jsonColumn != JsonColumn - || keyPropertyMap.Count != _keyPropertyMap.Count - || keyPropertyMap.Zip(_keyPropertyMap, (n, o) => n.Value != o.Value).Any(x => x) - || path != Path - ? new JsonQueryExpression(EntityType, jsonColumn, IsCollection, keyPropertyMap, Type, path, nullable) - : this; - - /// - public override bool Equals(object? obj) - => obj != null - && (ReferenceEquals(this, obj) - || obj is JsonQueryExpression jsonQueryExpression - && Equals(jsonQueryExpression)); - - private bool Equals(JsonQueryExpression jsonQueryExpression) - => EntityType.Equals(jsonQueryExpression.EntityType) - && JsonColumn.Equals(jsonQueryExpression.JsonColumn) - && IsCollection.Equals(jsonQueryExpression.IsCollection) - && Path.Equals(jsonQueryExpression.Path) - && IsNullable == jsonQueryExpression.IsNullable - && KeyPropertyMapEquals(jsonQueryExpression._keyPropertyMap); - - private bool KeyPropertyMapEquals(IReadOnlyDictionary other) + foreach (var (key, value) in _keyPropertyMap) { - if (_keyPropertyMap.Count != other.Count) + if (!other.TryGetValue(key, out var column) || !value.Equals(column)) { return false; } - - foreach (var (key, value) in _keyPropertyMap) - { - if (!other.TryGetValue(key, out var column) || !value.Equals(column)) - { - return false; - } - } - - return true; } - /// - public override int GetHashCode() - // not incorporating _keyPropertyMap into the hash, too much work - => HashCode.Combine(EntityType, JsonColumn, IsCollection, Path, IsNullable); + return true; } + + /// + public override int GetHashCode() + // not incorporating _keyPropertyMap into the hash, too much work + => HashCode.Combine(EntityType, JsonColumn, IsCollection, Path, IsNullable); } diff --git a/src/EFCore.Relational/Query/PathSegment.cs b/src/EFCore.Relational/Query/PathSegment.cs new file mode 100644 index 00000000000..8c25cd5eacd --- /dev/null +++ b/src/EFCore.Relational/Query/PathSegment.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.Query.SqlExpressions; + +namespace Microsoft.EntityFrameworkCore.Query; + +/// +/// +/// A class representing a component of JSON path used in or . +/// +/// +/// This type is typically used by database providers (and other extensions). It is generally +/// not used in application code. +/// +/// +public class PathSegment +{ + /// + /// Creates a new instance of the class. + /// + /// A key which is being accessed in the JSON. + public PathSegment(string key) + { + Key = key; + } + + /// + /// The key which is being accessed in the JSON. + /// + public virtual string Key { get; } + + /// + public override string ToString() => (Key == "$" ? "" : ".") + Key; + + /// + public override bool Equals(object? obj) + => obj != null + && (ReferenceEquals(this, obj) + || obj is PathSegment pathSegment + && Equals(pathSegment)); + + private bool Equals(PathSegment pathSegment) + => Key == pathSegment.Key; + + /// + public override int GetHashCode() + => HashCode.Combine(Key); +} diff --git a/src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs index e888c5f99eb..1a568204cf9 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs @@ -1,106 +1,106 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions +namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions; + +/// +/// +/// An expression representing a scalar extracted from a JSON column with the given path in SQL tree. +/// +/// +/// This type is typically used by database providers (and other extensions). It is generally +/// not used in application code. +/// +/// +public class JsonScalarExpression : SqlExpression { /// - /// Expression representing a scalar extracted from a JSON column with the given path. + /// Creates a new instance of the class. /// - public class JsonScalarExpression : SqlExpression + /// A column containg JSON value. + /// A property representing the result of this expression. + /// A list of path segments leading to the scalar from the root of the JSON stored in the column. + /// A value indicating whether the expression is nullable. + public JsonScalarExpression( + ColumnExpression jsonColumn, + IProperty property, + IReadOnlyList path, + bool nullable) + : this(jsonColumn, path, property.ClrType, property.FindRelationalTypeMapping()!, nullable) { - /// - /// Creates a new instance of the class. - /// - /// A column containg JSON. - /// A property representing the result of this expression. - /// A JSON path leading to the scalar from the root of the JSON stored in the column. - /// A value indicating whether the expression is nullable. - public JsonScalarExpression( - ColumnExpression jsonColumn, - IProperty property, - SqlExpression path, - bool nullable) - : this(jsonColumn, property.ClrType, property.FindRelationalTypeMapping()!, path, nullable) - { - } + } - internal JsonScalarExpression( - ColumnExpression jsonColumn, - Type type, - RelationalTypeMapping typeMapping, - SqlExpression path, - bool nullable) - : base(type, typeMapping) - { - JsonColumn = jsonColumn; - Path = path; - IsNullable = nullable; - } + internal JsonScalarExpression( + ColumnExpression jsonColumn, + IReadOnlyList path, + Type type, + RelationalTypeMapping typeMapping, + bool nullable) + : base(type, typeMapping) + { + JsonColumn = jsonColumn; + Path = path; + IsNullable = nullable; + } - /// - /// The column containg JSON. - /// - public virtual ColumnExpression JsonColumn { get; } + /// + /// The column containg JSON value. + /// + public virtual ColumnExpression JsonColumn { get; } - /// - /// The JSON path leading to the scalar from the root of the JSON stored in the column. - /// - public virtual SqlExpression Path { get; } + /// + /// The list of path segments leading to the scalar from the root of the JSON stored in the column. + /// + public virtual IReadOnlyList Path { get; } - /// - /// The value indicating whether the expression is nullable. - /// - public virtual bool IsNullable { get; } + /// + /// The value indicating whether the expression is nullable. + /// + public virtual bool IsNullable { get; } - /// - protected override Expression VisitChildren(ExpressionVisitor visitor) - { - var jsonColumn = (ColumnExpression)visitor.Visit(JsonColumn); - var jsonColumnMadeNullable = jsonColumn.IsNullable && !JsonColumn.IsNullable; + /// + protected override Expression VisitChildren(ExpressionVisitor visitor) + { + var jsonColumn = (ColumnExpression)visitor.Visit(JsonColumn); + var jsonColumnMadeNullable = jsonColumn.IsNullable && !JsonColumn.IsNullable; - return jsonColumn != JsonColumn - ? new JsonScalarExpression( - jsonColumn, - Type, - TypeMapping!, - Path, - IsNullable || jsonColumnMadeNullable) - : this; - } + // TODO Call update: Issue#28887 + return jsonColumn != JsonColumn + ? new JsonScalarExpression( + jsonColumn, + Path, + Type, + TypeMapping!, + IsNullable || jsonColumnMadeNullable) + : this; + } - /// - /// Creates a new expression that is like this one, but using the supplied children. If all of the children are the same, it will - /// return this expression. - /// - /// The property of the result. - /// The property of the result. - /// This expression if no children changed, or an expression with the updated children. - public virtual JsonScalarExpression Update( - ColumnExpression jsonColumn, - SqlExpression path) - => jsonColumn != JsonColumn - || path != Path - ? new JsonScalarExpression(jsonColumn, Type, TypeMapping!, path, IsNullable) - : this; + /// + /// Creates a new expression that is like this one, but using the supplied children. If all of the children are the same, it will + /// return this expression. + /// + /// The property of the result. + /// This expression if no children changed, or an expression with the updated children. + public virtual JsonScalarExpression Update(ColumnExpression jsonColumn) + => jsonColumn != JsonColumn + ? new JsonScalarExpression(jsonColumn, Path, Type, TypeMapping!, IsNullable) + : this; - /// - protected override void Print(ExpressionPrinter expressionPrinter) - { - expressionPrinter.Append("JsonScalarExpression(column: "); - expressionPrinter.Visit(JsonColumn); - expressionPrinter.Append(" Path: "); - expressionPrinter.Visit(Path); - expressionPrinter.Append(")"); - } + /// + protected override void Print(ExpressionPrinter expressionPrinter) + { + expressionPrinter.Append("JsonScalarExpression(column: "); + expressionPrinter.Visit(JsonColumn); + expressionPrinter.Append($", {string.Join("", Path.Select(e => e.ToString()))})"); + } - /// - public override bool Equals(object? obj) - => obj is JsonScalarExpression jsonScalarExpression - && JsonColumn.Equals(jsonScalarExpression.JsonColumn) - && Path.Equals(jsonScalarExpression.Path); + /// + public override bool Equals(object? obj) + => obj is JsonScalarExpression jsonScalarExpression + && JsonColumn.Equals(jsonScalarExpression.JsonColumn) + && Path.SequenceEqual(jsonScalarExpression.Path); - /// - public override int GetHashCode() - => HashCode.Combine(base.GetHashCode(), JsonColumn, Path); - } + /// + public override int GetHashCode() + => HashCode.Combine(base.GetHashCode(), JsonColumn, Path); } diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 6cb57eeebf1..72e6ff23af4 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -463,9 +463,9 @@ void GenerateNonHierarchyNonSplittingEntityType(ITableBase table, TableExpressio new JsonQueryExpression( targetEntityType, jsonColumn, - ownedJsonNavigation.IsCollection, keyPropertiesMap, - ownedJsonNavigation.ClrType), + ownedJsonNavigation.ClrType, + ownedJsonNavigation.IsCollection), !ownedJsonNavigation.ForeignKey.IsRequiredDependent); entityProjection.AddNavigationBinding(ownedJsonNavigation, entityShaperExpression); @@ -1391,7 +1391,7 @@ static Dictionary BuildJsonProjection { var ordered = projections .OrderBy(x => $"{x.JsonColumn.TableAlias}.{x.JsonColumn.Name}") - .ThenBy(x => BreakJsonPathIntoComponents(x.Path).Count); + .ThenBy(x => x.Path.Count); var needed = new List(); foreach (var orderedElement in ordered) @@ -1402,9 +1402,9 @@ static Dictionary BuildJsonProjection { jsonScalarExpression = new JsonScalarExpression( orderedElement.JsonColumn, + orderedElement.Path, orderedElement.JsonColumn.Type, orderedElement.JsonColumn.TypeMapping!, - orderedElement.Path, orderedElement.IsNullable); needed.Add(jsonScalarExpression); @@ -1442,9 +1442,9 @@ ConstantExpression AddJsonProjection(JsonQueryExpression jsonQueryExpression, Js var additionalPath = new string[0]; // this will be more tricky once we support more complicated json path options - additionalPath = BreakJsonPathIntoComponents(jsonQueryExpression.Path) - .Skip(BreakJsonPathIntoComponents(jsonScalarToAdd.Path).Count) - .Select(x => (string)((SqlConstantExpression)x).Value!) + additionalPath = jsonQueryExpression.Path + .Skip(jsonScalarToAdd.Path.Count) + .Select(x => x.Key) .ToArray(); var jsonColumnIndex = AddToProjection(jsonScalarToAdd); @@ -1490,8 +1490,8 @@ static bool JsonEntityContainedIn(JsonScalarExpression sourceExpression, JsonQue return false; } - var sourcePath = BreakJsonPathIntoComponents(sourceExpression.Path); - var targetPath = BreakJsonPathIntoComponents(targetExpression.Path); + var sourcePath = sourceExpression.Path; + var targetPath = targetExpression.Path; if (targetPath.Count < sourcePath.Count) { @@ -1500,21 +1500,6 @@ static bool JsonEntityContainedIn(JsonScalarExpression sourceExpression, JsonQue return sourcePath.SequenceEqual(targetPath.Take(sourcePath.Count)); } - - static List BreakJsonPathIntoComponents(SqlExpression jsonPath) - { - var result = new List(); - var currentPath = jsonPath; - while (currentPath is SqlBinaryExpression sqlBinary && sqlBinary.OperatorType == ExpressionType.Add) - { - result.Insert(0, sqlBinary.Right); - currentPath = sqlBinary.Left; - } - - result.Insert(0, currentPath); - - return result; - } } /// @@ -3443,9 +3428,9 @@ JsonQueryExpression LiftJsonQueryFromSubquery(JsonQueryExpression jsonQueryExpre { var jsonScalarExpression = new JsonScalarExpression( jsonQueryExpression.JsonColumn, + jsonQueryExpression.Path, jsonQueryExpression.JsonColumn.TypeMapping!.ClrType, jsonQueryExpression.JsonColumn.TypeMapping, - jsonQueryExpression.Path, jsonQueryExpression.IsNullable); var newJsonColumn = subquery.GenerateOuterColumn(subqueryTableReferenceExpression, jsonScalarExpression); @@ -3467,11 +3452,12 @@ JsonQueryExpression LiftJsonQueryFromSubquery(JsonQueryExpression jsonQueryExpre } // clear up the json path - we start from empty path after pushdown - return jsonQueryExpression.Update( + return new JsonQueryExpression( + jsonQueryExpression.EntityType, newJsonColumn, newKeyPropertyMap, - path: new SqlConstantExpression(Constant("$"), typeMapping: null), - newJsonColumn.IsNullable); + jsonQueryExpression.Type, + jsonQueryExpression.IsCollection); } } diff --git a/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs b/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs index 898b1e6fe22..0db76ba9fd7 100644 --- a/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs +++ b/src/EFCore.SqlServer/Query/Internal/SearchConditionConvertingExpressionVisitor.cs @@ -761,13 +761,5 @@ protected override Expression VisitUpdate(UpdateExpression updateExpression) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExpression) - { - var parentSearchCondition = _isSearchCondition; - _isSearchCondition = false; - var jsonPath = (SqlExpression)Visit(jsonScalarExpression.Path); - _isSearchCondition = parentSearchCondition; - - return jsonScalarExpression.Update(jsonScalarExpression.JsonColumn, jsonPath); - } + protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExpression) => jsonScalarExpression; } diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs index d7139c54d24..ffda9ec8a46 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Text.Json; -using Microsoft.EntityFrameworkCore.Query; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal; @@ -309,21 +308,7 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp Visit(jsonScalarExpression.JsonColumn); - var jsonPathStrings = new List(); - - if (jsonScalarExpression.Path != null) - { - var currentPath = jsonScalarExpression.Path; - while (currentPath is SqlBinaryExpression sqlBinary && sqlBinary.OperatorType == ExpressionType.Add) - { - currentPath = sqlBinary.Left; - jsonPathStrings.Insert(0, (string)((SqlConstantExpression)sqlBinary.Right).Value!); - } - - jsonPathStrings.Insert(0, (string)((SqlConstantExpression)currentPath).Value!); - } - - Sql.Append($",'{string.Join(".", jsonPathStrings)}')"); + Sql.Append($",'{string.Join("", jsonScalarExpression.Path.Select(e => e.ToString()))}')"); if (jsonScalarExpression.Type != typeof(JsonElement)) {