diff --git a/src/EFCore.Relational/Query/JsonQueryExpression.cs b/src/EFCore.Relational/Query/JsonQueryExpression.cs index 783f55756a9..2aade3bd5f4 100644 --- a/src/EFCore.Relational/Query/JsonQueryExpression.cs +++ b/src/EFCore.Relational/Query/JsonQueryExpression.cs @@ -36,7 +36,7 @@ public JsonQueryExpression( entityType, jsonColumn, keyPropertyMap, - path: new List { new("$") }, + path: Array.Empty(), type, collection, jsonColumn.IsNullable) @@ -169,9 +169,7 @@ public virtual JsonQueryExpression BindCollectionElement(SqlExpression collectio { // this needs to be changed IF JsonQueryExpression will also be used for collection of primitives // see issue #28688 - Check.DebugAssert( - Path.Last().ArrayIndex == null, - "Already accessing JSON array element."); + Check.DebugAssert(Path.Count == 0 || Path[^1].ArrayIndex == null, "Already accessing JSON array element."); var newPath = Path.ToList(); newPath.Add(new PathSegment(collectionIndexExpression)); @@ -215,7 +213,7 @@ public virtual void Print(ExpressionPrinter expressionPrinter) { expressionPrinter.Append("JsonQueryExpression("); expressionPrinter.Visit(JsonColumn); - expressionPrinter.Append($", {string.Join("", Path.Select(e => e.ToString()))})"); + expressionPrinter.Append($""", "{string.Join(".", Path.Select(e => e.ToString()))}")"""); } /// @@ -282,6 +280,6 @@ private bool KeyPropertyMapEquals(IReadOnlyDictionary public override int GetHashCode() - // not incorporating _keyPropertyMap into the hash, too much work + // 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 index 25ac61e9026..32b680bd7d2 100644 --- a/src/EFCore.Relational/Query/PathSegment.cs +++ b/src/EFCore.Relational/Query/PathSegment.cs @@ -48,8 +48,7 @@ public PathSegment(SqlExpression arrayIndex) /// public override string ToString() - { - var arrayIndex = ArrayIndex switch + => PropertyName ?? ArrayIndex switch { null => "", SqlConstantExpression { Value: not null } sqlConstant => $"[{sqlConstant.Value}]", @@ -57,9 +56,6 @@ public override string ToString() _ => "[(...)]" }; - return (PropertyName == "$" ? "" : ".") + (PropertyName ?? arrayIndex); - } - /// public override bool Equals(object? obj) => obj is PathSegment pathSegment && Equals(pathSegment); diff --git a/src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs index 7629c0c1d7e..920f28efb73 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs @@ -91,7 +91,7 @@ protected override void Print(ExpressionPrinter expressionPrinter) { expressionPrinter.Append("JsonScalarExpression(column: "); expressionPrinter.Visit(JsonColumn); - expressionPrinter.Append($", {string.Join("", Path.Select(e => e.ToString()))})"); + expressionPrinter.Append($""", "{string.Join(".", Path.Select(e => e.ToString()))}")"""); } /// diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index e39a89f4ad5..5fcac633754 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -1597,7 +1597,7 @@ static Dictionary BuildJsonProjection var ordered = projections .OrderBy(x => $"{x.JsonColumn.TableAlias}.{x.JsonColumn.Name}") .ThenBy(x => x.Path.Count) - .ThenBy(x => x.Path[^1].ArrayIndex != null); + .ThenBy(x => x.Path.Count > 0 && x.Path[^1].ArrayIndex != null); var needed = new List(); foreach (var orderedElement in ordered) diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs index 3f7fd0bc9eb..0cd02c656e4 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs @@ -318,11 +318,11 @@ when tableExpression.FindAnnotation(SqlServerAnnotationNames.TemporalOperationTy /// protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExpression) { - if (jsonScalarExpression.Path.Count == 1 - && jsonScalarExpression.Path[0].ToString() == "$") + // TODO: Stop producing empty JsonScalarExpressions, #30768 + var path = jsonScalarExpression.Path; + if (path.Count == 0) { Visit(jsonScalarExpression.JsonColumn); - return jsonScalarExpression; } @@ -340,33 +340,38 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp Visit(jsonScalarExpression.JsonColumn); - Sql.Append(", '"); + Sql.Append(", '$"); foreach (var pathSegment in jsonScalarExpression.Path) { - if (pathSegment.PropertyName != null) + switch (pathSegment) { - Sql.Append((pathSegment.PropertyName == "$" ? "" : ".") + pathSegment.PropertyName); + case { PropertyName: string propertyName }: + Sql.Append(".").Append(propertyName); + break; + + case { ArrayIndex: SqlExpression arrayIndex }: + Sql.Append("["); + + if (arrayIndex is SqlConstantExpression) + { + Visit(pathSegment.ArrayIndex); + } + else + { + Sql.Append("' + CAST("); + Visit(arrayIndex); + Sql.Append(" AS "); + Sql.Append(_typeMappingSource.GetMapping(typeof(string)).StoreType); + Sql.Append(") + '"); + } + + Sql.Append("]"); + break; + + default: + throw new ArgumentOutOfRangeException(); } - if (pathSegment.ArrayIndex != null) - { - Sql.Append("["); - - if (pathSegment.ArrayIndex is SqlConstantExpression) - { - Visit(pathSegment.ArrayIndex); - } - else - { - Sql.Append("' + CAST("); - Visit(pathSegment.ArrayIndex); - Sql.Append(" AS "); - Sql.Append(_typeMappingSource.GetMapping(typeof(string)).StoreType); - Sql.Append(") + '"); - } - - Sql.Append("]"); - } } Sql.Append("')"); diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs index db25b86d71b..66db40fa00c 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs +++ b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs @@ -126,13 +126,8 @@ private Expression VisitRegexp(RegexpExpression regexpExpression) /// protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExpression) { - // TODO: This trims the leading $ PathSegment, which isn't actually part of the path (but rather path of the JSONPATH language - // used to generate the path in SQL for *some* databases). - // Instead, we should not be producing JsonScalarExpression at all with the leading $. - var path = jsonScalarExpression.Path is [{ PropertyName: "$" }, ..] - ? jsonScalarExpression.Path.Skip(1).ToArray() - : jsonScalarExpression.Path; - + // TODO: Stop producing empty JsonScalarExpressions, #30768 + var path = jsonScalarExpression.Path; if (path.Count == 0) { Visit(jsonScalarExpression.JsonColumn); diff --git a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs index 96fe159db2d..b31cf584ca8 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs @@ -761,7 +761,7 @@ public virtual async Task Json_collection_element_access_in_projection_using_unt ss => ss.Set().Select(x => x.OwnedCollectionRoot[MyMethod(x.Id)]).AsNoTracking()))).Message; Assert.Equal( - CoreStrings.TranslationFailed("JsonQueryExpression(j.OwnedCollectionRoot, $)"), + CoreStrings.TranslationFailed("""JsonQueryExpression(j.OwnedCollectionRoot, "")"""), message); } @@ -775,7 +775,7 @@ public virtual async Task Json_collection_element_access_in_projection_using_unt ss => ss.Set().Select(x => x.OwnedCollectionRoot[0].OwnedReferenceBranch.OwnedCollectionLeaf[MyMethod(x.Id)]).AsNoTracking()))).Message; Assert.Equal( - CoreStrings.TranslationFailed("JsonQueryExpression(j.OwnedCollectionRoot, $.[0].OwnedReferenceBranch.OwnedCollectionLeaf)"), + CoreStrings.TranslationFailed("""JsonQueryExpression(j.OwnedCollectionRoot, "[0].OwnedReferenceBranch.OwnedCollectionLeaf")"""), message); } @@ -967,7 +967,7 @@ public virtual async Task Json_collection_element_access_manual_Element_at_and_p CollectionElement = x.OwnedCollectionRoot[prm].OwnedCollectionBranch.Select(xx => "Foo").ElementAt(0) })))).Message; - Assert.Equal(CoreStrings.TranslationFailed("JsonQueryExpression(j.OwnedCollectionRoot, $.[__prm_0].OwnedCollectionBranch)"), message); + Assert.Equal(CoreStrings.TranslationFailed("""JsonQueryExpression(j.OwnedCollectionRoot, "[__prm_0].OwnedCollectionBranch")"""), message); } [ConditionalTheory] @@ -984,7 +984,7 @@ public virtual async Task Json_collection_element_access_manual_Element_at_and_p CollectionElement = x.OwnedCollectionRoot[prm + x.Id].OwnedCollectionBranch.Select(xx => x.Id).ElementAt(0) })))).Message; - Assert.Equal(CoreStrings.TranslationFailed("JsonQueryExpression(j.OwnedCollectionRoot, $.[(...)].OwnedCollectionBranch)"), message); + Assert.Equal(CoreStrings.TranslationFailed("""JsonQueryExpression(j.OwnedCollectionRoot, "[(...)].OwnedCollectionBranch")"""), message); } [ConditionalTheory] @@ -1000,7 +1000,7 @@ public virtual async Task Json_collection_element_access_manual_Element_at_and_p CollectionElement = x.OwnedCollectionRoot.Select(xx => x.OwnedReferenceRoot).ElementAt(0) })))).Message; - Assert.Equal(CoreStrings.TranslationFailed("JsonQueryExpression(j.OwnedCollectionRoot, $)"), message); + Assert.Equal(CoreStrings.TranslationFailed("""JsonQueryExpression(j.OwnedCollectionRoot, "")"""), message); } [ConditionalTheory] @@ -1016,7 +1016,7 @@ public virtual async Task Json_collection_element_access_manual_Element_at_and_p CollectionElement = x.OwnedCollectionRoot.Select(xx => x.OwnedCollectionRoot).ElementAt(0) })))).Message; - Assert.Equal(CoreStrings.TranslationFailed("JsonQueryExpression(j.OwnedCollectionRoot, $)"), message); + Assert.Equal(CoreStrings.TranslationFailed("""JsonQueryExpression(j.OwnedCollectionRoot, "")"""), message); } [ConditionalTheory] @@ -1032,7 +1032,7 @@ public virtual async Task Json_collection_element_access_manual_Element_at_and_p CollectionElement = x.OwnedCollectionRoot.Select(xx => new { xx.OwnedReferenceBranch }).ElementAt(0) })))).Message; - Assert.Equal(CoreStrings.TranslationFailed("JsonQueryExpression(j.OwnedCollectionRoot, $)"), message); + Assert.Equal(CoreStrings.TranslationFailed("""JsonQueryExpression(j.OwnedCollectionRoot, "")"""), message); } [ConditionalTheory] @@ -1048,7 +1048,7 @@ public virtual async Task Json_collection_element_access_manual_Element_at_and_p CollectionElement = x.OwnedCollectionRoot.Select(xx => new JsonEntityBasic { Id = x.Id }).ElementAt(0) })))).Message; - Assert.Equal(CoreStrings.TranslationFailed("JsonQueryExpression(j.OwnedCollectionRoot, $)"), message); + Assert.Equal(CoreStrings.TranslationFailed("""JsonQueryExpression(j.OwnedCollectionRoot, "")"""), message); } [ConditionalTheory]