From 2a56df421e872d2c83879c0666a4c051b9ddd0b5 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 23 May 2023 22:40:24 +0200 Subject: [PATCH 1/2] Always convert to parameter query roots in relational --- .../Query/Internal/ContainsTranslator.cs | 3 + .../Query/RelationalQueryRootProcessor.cs | 11 +++- ...yableMethodTranslatingExpressionVisitor.cs | 53 ++++++++++----- .../SqlServerServiceCollectionExtensions.cs | 1 - .../Properties/SqlServerStrings.Designer.cs | 8 +++ .../Properties/SqlServerStrings.resx | 3 + .../Internal/SqlServerQueryRootProcessor.cs | 44 ------------- .../SqlServerQueryTranslationPreprocessor.cs | 41 ------------ ...rverQueryTranslationPreprocessorFactory.cs | 53 --------------- ...yableMethodTranslatingExpressionVisitor.cs | 19 +++++- ...thodTranslatingExpressionVisitorFactory.cs | 11 +++- .../Internal/SqlServerTypeMappingSource.cs | 12 +--- .../SqliteServiceCollectionExtensions.cs | 1 - .../Properties/SqliteStrings.Designer.cs | 8 +++ .../Properties/SqliteStrings.resx | 3 + .../Internal/SqliteQueryRootProcessor.cs | 44 ------------- .../SqliteQueryTranslationPreprocessor.cs | 37 ----------- ...liteQueryTranslationPreprocessorFactory.cs | 46 ------------- ...yableMethodTranslatingExpressionVisitor.cs | 17 ++++- .../Internal/SqliteTypeMappingSource.cs | 41 +++++------- .../Query/QueryTranslationPreprocessor.cs | 2 +- ...yableMethodTranslatingExpressionVisitor.cs | 66 +++++++++++++------ ...imitiveCollectionsQueryOldSqlServerTest.cs | 57 ++++++++-------- 23 files changed, 210 insertions(+), 371 deletions(-) delete mode 100644 src/EFCore.SqlServer/Query/Internal/SqlServerQueryRootProcessor.cs delete mode 100644 src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPreprocessor.cs delete mode 100644 src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPreprocessorFactory.cs delete mode 100644 src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryRootProcessor.cs delete mode 100644 src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPreprocessor.cs delete mode 100644 src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPreprocessorFactory.cs diff --git a/src/EFCore.Relational/Query/Internal/ContainsTranslator.cs b/src/EFCore.Relational/Query/Internal/ContainsTranslator.cs index 3406c8fd24a..27ec3e4176a 100644 --- a/src/EFCore.Relational/Query/Internal/ContainsTranslator.cs +++ b/src/EFCore.Relational/Query/Internal/ContainsTranslator.cs @@ -39,6 +39,9 @@ public ContainsTranslator(ISqlExpressionFactory sqlExpressionFactory) IReadOnlyList arguments, IDiagnosticsLogger logger) { + // Note that almost all forms of Contains are queryable (e.g. over inline/parameter collections), and translated in + // RelationalQueryableMethodTranslatingExpressionVisitor.TranslateContains. + // This enumerable Contains translation is still needed for entity Contains (#30712) SqlExpression? itemExpression = null, valuesExpression = null; // Identify static Enumerable.Contains and instance List.Contains diff --git a/src/EFCore.Relational/Query/RelationalQueryRootProcessor.cs b/src/EFCore.Relational/Query/RelationalQueryRootProcessor.cs index 10ba014ce94..4dd48c4c6a3 100644 --- a/src/EFCore.Relational/Query/RelationalQueryRootProcessor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryRootProcessor.cs @@ -28,11 +28,20 @@ public RelationalQueryRootProcessor( /// /// Indicates that a can be converted to a ; - /// this will later be translated to a SQL . + /// the latter will end up in for + /// translation to a SQL . /// protected override bool ShouldConvertToInlineQueryRoot(NewArrayExpression newArrayExpression) => true; + /// + /// Indicates that a can be converted to a ; + /// the latter will end up in for + /// translation to a provider-specific SQL expansion mechanism, e.g. OPENJSON on SQL Server. + /// + protected override bool ShouldConvertToParameterQueryRoot(ParameterExpression constantExpression) + => true; + /// protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression) { diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index 727a54681af..192717d23f8 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -75,7 +75,7 @@ protected RelationalQueryableMethodTranslatingExpressionVisitor( /// public override Expression Translate(Expression expression) { - var visited = Visit(expression); + var visited = base.Translate(expression); if (!_subquery) { @@ -256,25 +256,44 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp var translated = base.VisitMethodCall(methodCallExpression); - // Attempt to translate access into a primitive collection property - if (translated == QueryCompilationContext.NotTranslatedExpression - && _sqlTranslator.TryTranslatePropertyAccess(methodCallExpression, out var propertyAccessExpression) - && propertyAccessExpression is - { - TypeMapping.ElementTypeMapping: RelationalTypeMapping elementTypeMapping - } collectionPropertyAccessExpression) + if (translated == QueryCompilationContext.NotTranslatedExpression) { - var tableAlias = collectionPropertyAccessExpression switch + // Attempt to translate access into a primitive collection property (i.e. array column) + if (_sqlTranslator.TryTranslatePropertyAccess(methodCallExpression, out var propertyAccessExpression) + && propertyAccessExpression is + { + TypeMapping.ElementTypeMapping: RelationalTypeMapping elementTypeMapping + } collectionPropertyAccessExpression) { - ColumnExpression c => c.Name[..1].ToLowerInvariant(), - JsonScalarExpression { Path: [.., { PropertyName: string propertyName }] } => propertyName[..1].ToLowerInvariant(), - _ => "j" - }; + var tableAlias = collectionPropertyAccessExpression switch + { + ColumnExpression c => c.Name[..1].ToLowerInvariant(), + JsonScalarExpression { Path: [.., { PropertyName: string propertyName }] } => propertyName[..1].ToLowerInvariant(), + _ => "j" + }; - if (TranslateCollection(collectionPropertyAccessExpression, elementTypeMapping, tableAlias) is - { } primitiveCollectionTranslation) - { - return primitiveCollectionTranslation; + if (TranslateCollection(collectionPropertyAccessExpression, elementTypeMapping, tableAlias) is + { } primitiveCollectionTranslation) + { + return primitiveCollectionTranslation; + } + } + + // For Contains over a collection parameter, if the provider hasn't implemented TranslateCollection (e.g. OPENJSON on SQL + // Server), we need to fall back to the previous IN translation. + if (method.IsGenericMethod + && method.GetGenericMethodDefinition() == QueryableMethods.Contains + && methodCallExpression.Arguments[0] is ParameterQueryRootExpression parameterSource + && TranslateExpression(methodCallExpression.Arguments[1]) is SqlExpression item + && _sqlTranslator.Visit(parameterSource.ParameterExpression) is SqlParameterExpression sqlParameterExpression) + { + var inExpression = _sqlExpressionFactory.In(item, sqlParameterExpression); + var selectExpression = new SelectExpression(inExpression); + var shaperExpression = Expression.Convert( + new ProjectionBindingExpression(selectExpression, new ProjectionMember(), typeof(bool?)), typeof(bool)); + var shapedQueryExpression = new ShapedQueryExpression(selectExpression, shaperExpression) + .UpdateResultCardinality(ResultCardinality.Single); + return shapedQueryExpression; } } diff --git a/src/EFCore.SqlServer/Extensions/SqlServerServiceCollectionExtensions.cs b/src/EFCore.SqlServer/Extensions/SqlServerServiceCollectionExtensions.cs index 536f658af60..a994c313168 100644 --- a/src/EFCore.SqlServer/Extensions/SqlServerServiceCollectionExtensions.cs +++ b/src/EFCore.SqlServer/Extensions/SqlServerServiceCollectionExtensions.cs @@ -120,7 +120,6 @@ public static IServiceCollection AddEntityFrameworkSqlServer(this IServiceCollec .TryAdd() .TryAdd() .TryAdd() - .TryAdd() .TryAdd() .TryAdd() .TryAdd() diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 46e0a04d6b4..f82784ab5d3 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -43,6 +43,14 @@ public static string CannotProduceUnterminatedSQLWithComments(object? operation) GetString("CannotProduceUnterminatedSQLWithComments", nameof(operation)), operation); + /// + /// EF Core's SQL Server compatibility level is set to {compatibilityLevel}; compatibility level 130 (SQL Server 2016) is the minimum for most forms of querying of JSON arrays. + /// + public static string CompatibilityLevelTooLowForScalarCollections(object? compatibilityLevel) + => string.Format( + GetString("CompatibilityLevelTooLowForScalarCollections", nameof(compatibilityLevel)), + compatibilityLevel); + /// /// '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured with different identity increment values. /// diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index 11e2e6c66a4..114cd64c8fb 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -126,6 +126,9 @@ Can't produce unterminated SQL with comments when generating migrations SQL for {operation}. + + EF Core's SQL Server compatibility level is set to {compatibilityLevel}; compatibility level 130 (SQL Server 2016) is the minimum for most forms of querying of JSON arrays. + '{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured with different identity increment values. diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryRootProcessor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryRootProcessor.cs deleted file mode 100644 index eaff1e7eb80..00000000000 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryRootProcessor.cs +++ /dev/null @@ -1,44 +0,0 @@ -// 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.SqlServer.Infrastructure.Internal; - -namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; - -/// -/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to -/// the same compatibility standards as public APIs. It may be changed or removed without notice in -/// 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. -/// -public class SqlServerQueryRootProcessor : RelationalQueryRootProcessor -{ - private readonly bool _supportsOpenJson; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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. - /// - public SqlServerQueryRootProcessor( - QueryTranslationPreprocessorDependencies dependencies, - RelationalQueryTranslationPreprocessorDependencies relationalDependencies, - QueryCompilationContext queryCompilationContext, - ISqlServerSingletonOptions sqlServerSingletonOptions) - : base(dependencies, relationalDependencies, queryCompilationContext) - => _supportsOpenJson = sqlServerSingletonOptions.CompatibilityLevel >= 130; - - /// - /// Indicates that a can be converted to a , if the - /// configured SQL Server version supports OPENJSON. - /// - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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 bool ShouldConvertToParameterQueryRoot(ParameterExpression parameterExpression) - => _supportsOpenJson; -} diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPreprocessor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPreprocessor.cs deleted file mode 100644 index f22ddc0236a..00000000000 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPreprocessor.cs +++ /dev/null @@ -1,41 +0,0 @@ -// 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.SqlServer.Infrastructure.Internal; - -namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; - -/// -/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to -/// the same compatibility standards as public APIs. It may be changed or removed without notice in -/// 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. -/// -public class SqlServerQueryTranslationPreprocessor : RelationalQueryTranslationPreprocessor -{ - private readonly ISqlServerSingletonOptions _sqlServerSingletonOptions; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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. - /// - public SqlServerQueryTranslationPreprocessor( - QueryTranslationPreprocessorDependencies dependencies, - RelationalQueryTranslationPreprocessorDependencies relationalDependencies, - ISqlServerSingletonOptions sqlServerSingletonOptions, - QueryCompilationContext queryCompilationContext) - : base(dependencies, relationalDependencies, queryCompilationContext) - => _sqlServerSingletonOptions = sqlServerSingletonOptions; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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 ProcessQueryRoots(Expression expression) - => new SqlServerQueryRootProcessor(Dependencies, RelationalDependencies, QueryCompilationContext, _sqlServerSingletonOptions) - .Visit(expression); -} diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPreprocessorFactory.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPreprocessorFactory.cs deleted file mode 100644 index ca3f6d16d68..00000000000 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPreprocessorFactory.cs +++ /dev/null @@ -1,53 +0,0 @@ -// 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.SqlServer.Infrastructure.Internal; - -namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; - -/// -/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to -/// the same compatibility standards as public APIs. It may be changed or removed without notice in -/// 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. -/// -public class SqlServerQueryTranslationPreprocessorFactory : IQueryTranslationPreprocessorFactory -{ - private readonly ISqlServerSingletonOptions _sqlServerSingletonOptions; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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. - /// - public SqlServerQueryTranslationPreprocessorFactory( - QueryTranslationPreprocessorDependencies dependencies, - RelationalQueryTranslationPreprocessorDependencies relationalDependencies, - ISqlServerSingletonOptions sqlServerSingletonOptions) - { - Dependencies = dependencies; - RelationalDependencies = relationalDependencies; - _sqlServerSingletonOptions = sqlServerSingletonOptions; - } - - /// - /// Dependencies for this service. - /// - protected virtual QueryTranslationPreprocessorDependencies Dependencies { get; } - - /// - /// Relational provider-specific dependencies for this service. - /// - protected virtual RelationalQueryTranslationPreprocessorDependencies RelationalDependencies { get; } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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. - /// - public virtual QueryTranslationPreprocessor Create(QueryCompilationContext queryCompilationContext) - => new SqlServerQueryTranslationPreprocessor( - Dependencies, RelationalDependencies, _sqlServerSingletonOptions, queryCompilationContext); -} diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs index 45db664494c..63ea9ea02a1 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitor.cs @@ -3,6 +3,8 @@ using System.Diagnostics.CodeAnalysis; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; +using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; +using Microsoft.EntityFrameworkCore.SqlServer.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal; @@ -19,6 +21,7 @@ public class SqlServerQueryableMethodTranslatingExpressionVisitor : RelationalQu private readonly QueryCompilationContext _queryCompilationContext; private readonly IRelationalTypeMappingSource _typeMappingSource; private readonly ISqlExpressionFactory _sqlExpressionFactory; + private readonly int _sqlServerCompatibilityLevel; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -29,12 +32,15 @@ public class SqlServerQueryableMethodTranslatingExpressionVisitor : RelationalQu public SqlServerQueryableMethodTranslatingExpressionVisitor( QueryableMethodTranslatingExpressionVisitorDependencies dependencies, RelationalQueryableMethodTranslatingExpressionVisitorDependencies relationalDependencies, - QueryCompilationContext queryCompilationContext) + QueryCompilationContext queryCompilationContext, + ISqlServerSingletonOptions sqlServerSingletonOptions) : base(dependencies, relationalDependencies, queryCompilationContext) { _queryCompilationContext = queryCompilationContext; _typeMappingSource = relationalDependencies.TypeMappingSource; _sqlExpressionFactory = relationalDependencies.SqlExpressionFactory; + + _sqlServerCompatibilityLevel = sqlServerSingletonOptions.CompatibilityLevel; } /// @@ -50,6 +56,8 @@ protected SqlServerQueryableMethodTranslatingExpressionVisitor( _queryCompilationContext = parentVisitor._queryCompilationContext; _typeMappingSource = parentVisitor._typeMappingSource; _sqlExpressionFactory = parentVisitor._sqlExpressionFactory; + + _sqlServerCompatibilityLevel = parentVisitor._sqlServerCompatibilityLevel; } /// @@ -117,11 +125,18 @@ protected override Expression VisitExtension(Expression extensionExpression) /// 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 ShapedQueryExpression TranslateCollection( + protected override ShapedQueryExpression? TranslateCollection( SqlExpression sqlExpression, RelationalTypeMapping? elementTypeMapping, string tableAlias) { + if (_sqlServerCompatibilityLevel < 130) + { + AddTranslationErrorDetails(SqlServerStrings.CompatibilityLevelTooLowForScalarCollections(_sqlServerCompatibilityLevel)); + + return null; + } + // Generate the OPENJSON function expression, and wrap it in a SelectExpression. // Note that where the elementTypeMapping is known (i.e. collection columns), we immediately generate OPENJSON with a WITH clause diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitorFactory.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitorFactory.cs index fd0ffef35fc..37389e4edf2 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitorFactory.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQueryableMethodTranslatingExpressionVisitorFactory.cs @@ -1,6 +1,8 @@ // 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.SqlServer.Infrastructure.Internal; + namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; /// @@ -11,6 +13,8 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal; /// public class SqlServerQueryableMethodTranslatingExpressionVisitorFactory : IQueryableMethodTranslatingExpressionVisitorFactory { + private readonly ISqlServerSingletonOptions _sqlServerSingletonOptions; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -19,10 +23,12 @@ public class SqlServerQueryableMethodTranslatingExpressionVisitorFactory : IQuer /// public SqlServerQueryableMethodTranslatingExpressionVisitorFactory( QueryableMethodTranslatingExpressionVisitorDependencies dependencies, - RelationalQueryableMethodTranslatingExpressionVisitorDependencies relationalDependencies) + RelationalQueryableMethodTranslatingExpressionVisitorDependencies relationalDependencies, + ISqlServerSingletonOptions sqlServerSingletonOptions) { Dependencies = dependencies; RelationalDependencies = relationalDependencies; + _sqlServerSingletonOptions = sqlServerSingletonOptions; } /// @@ -42,5 +48,6 @@ public SqlServerQueryableMethodTranslatingExpressionVisitorFactory( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual QueryableMethodTranslatingExpressionVisitor Create(QueryCompilationContext queryCompilationContext) - => new SqlServerQueryableMethodTranslatingExpressionVisitor(Dependencies, RelationalDependencies, queryCompilationContext); + => new SqlServerQueryableMethodTranslatingExpressionVisitor( + Dependencies, RelationalDependencies, queryCompilationContext, _sqlServerSingletonOptions); } diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs index c08810d8318..10f7476b00f 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs @@ -493,16 +493,10 @@ public SqlServerTypeMappingSource( stringTypeMapping = (SqlServerStringTypeMapping)stringTypeMapping .Clone(new CollectionToJsonStringConverter(mappingInfo.ClrType, elementTypeMapping)); - // OPENJSON was introduced in SQL Server 2016 (compatibility level 130). If the user configures an older compatibility level, - // we allow mapping the column, but don't set the element type mapping on the mapping, so that it isn't queryable. - // This causes us to go into the old translation path for Contains over parameter via IN with constants. - if (_supportsOpenJson) + // The JSON representation for new[] { 1, 2 } is AQI= (base64), this cannot simply be cast to varbinary(max) (0x0102) + if (elementTypeMapping is not SqlServerByteArrayTypeMapping) { - // The JSON representation for new[] { 1, 2 } is AQI= (base64?), this cannot simply be cast to varbinary(max) (0x0102) - if (elementTypeMapping is not SqlServerByteArrayTypeMapping) - { - stringTypeMapping = (SqlServerStringTypeMapping)stringTypeMapping.CloneWithElementTypeMapping(elementTypeMapping); - } + stringTypeMapping = (SqlServerStringTypeMapping)stringTypeMapping.CloneWithElementTypeMapping(elementTypeMapping); } return stringTypeMapping; diff --git a/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs b/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs index 2e5e0301880..7145fe4c278 100644 --- a/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs +++ b/src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs @@ -109,7 +109,6 @@ public static IServiceCollection AddEntityFrameworkSqlite(this IServiceCollectio .TryAdd() .TryAdd() .TryAdd() - .TryAdd() .TryAdd() .TryAdd() .TryAdd() diff --git a/src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs b/src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs index 7d8d5beba71..16b89ac2248 100644 --- a/src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs +++ b/src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs @@ -67,6 +67,14 @@ public static string InvalidMigrationOperation(object? operation) GetString("InvalidMigrationOperation", nameof(operation)), operation); + /// + /// SQLite version {sqliteVersion} is being used, but version 3.38.0 or higher is required for querying into JSON collections. + /// + public static string QueryingIntoJsonCollectionsNotSupported(object? sqliteVersion) + => string.Format( + GetString("QueryingIntoJsonCollectionsNotSupported", nameof(sqliteVersion)), + sqliteVersion); + /// /// Generating idempotent scripts for migrations is not currently supported for SQLite. See https://go.microsoft.com/fwlink/?LinkId=723262 for more information and examples. /// diff --git a/src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx b/src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx index bf8895f442b..f34ef8c7022 100644 --- a/src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx +++ b/src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx @@ -135,6 +135,9 @@ SQLite does not support this migration operation ('{operation}'). See https://go.microsoft.com/fwlink/?LinkId=723262 for more information and examples. + + SQLite version {sqliteVersion} is being used, but version 3.38.0 or higher is required for querying into JSON collections. + The entity type '{entityType}' has composite key '{key}' which is configured to use generated values. SQLite does not support generated values on composite keys. Warning SqliteEventId.CompositeKeyWithValueGeneration string? string? diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryRootProcessor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryRootProcessor.cs deleted file mode 100644 index 15abafb0867..00000000000 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryRootProcessor.cs +++ /dev/null @@ -1,44 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.Data.Sqlite; - -namespace Microsoft.EntityFrameworkCore.Sqlite.Query.Internal; - -/// -/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to -/// the same compatibility standards as public APIs. It may be changed or removed without notice in -/// 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. -/// -public class SqliteQueryRootProcessor : RelationalQueryRootProcessor -{ - private readonly bool _areJsonFunctionsSupported; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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. - /// - public SqliteQueryRootProcessor( - QueryTranslationPreprocessorDependencies dependencies, - RelationalQueryTranslationPreprocessorDependencies relationalDependencies, - QueryCompilationContext queryCompilationContext) - : base(dependencies, relationalDependencies, queryCompilationContext) - => _areJsonFunctionsSupported = new Version(new SqliteConnection().ServerVersion) >= new Version(3, 38); - - - /// - /// Indicates that a can be converted to a , if the - /// configured SQL Server version supports JSON functions (json_each). - /// - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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 bool ShouldConvertToParameterQueryRoot(ParameterExpression parameterExpression) - => _areJsonFunctionsSupported; -} diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPreprocessor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPreprocessor.cs deleted file mode 100644 index 214787bd6f2..00000000000 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPreprocessor.cs +++ /dev/null @@ -1,37 +0,0 @@ -// 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.Sqlite.Query.Internal; - -/// -/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to -/// the same compatibility standards as public APIs. It may be changed or removed without notice in -/// 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. -/// -public class SqliteQueryTranslationPreprocessor : RelationalQueryTranslationPreprocessor -{ - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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. - /// - public SqliteQueryTranslationPreprocessor( - QueryTranslationPreprocessorDependencies dependencies, - RelationalQueryTranslationPreprocessorDependencies relationalDependencies, - QueryCompilationContext queryCompilationContext) - : base(dependencies, relationalDependencies, queryCompilationContext) - { - } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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 ProcessQueryRoots(Expression expression) - => new SqliteQueryRootProcessor(Dependencies, RelationalDependencies, QueryCompilationContext) - .Visit(expression); -} diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPreprocessorFactory.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPreprocessorFactory.cs deleted file mode 100644 index e7a8c021558..00000000000 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryTranslationPreprocessorFactory.cs +++ /dev/null @@ -1,46 +0,0 @@ -// 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.Sqlite.Query.Internal; - -/// -/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to -/// the same compatibility standards as public APIs. It may be changed or removed without notice in -/// 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. -/// -public class SqliteQueryTranslationPreprocessorFactory : IQueryTranslationPreprocessorFactory -{ - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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. - /// - public SqliteQueryTranslationPreprocessorFactory( - QueryTranslationPreprocessorDependencies dependencies, - RelationalQueryTranslationPreprocessorDependencies relationalDependencies) - { - Dependencies = dependencies; - RelationalDependencies = relationalDependencies; - } - - /// - /// Dependencies for this service. - /// - protected virtual QueryTranslationPreprocessorDependencies Dependencies { get; } - - /// - /// Relational provider-specific dependencies for this service. - /// - protected virtual RelationalQueryTranslationPreprocessorDependencies RelationalDependencies { get; } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// 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. - /// - public virtual QueryTranslationPreprocessor Create(QueryCompilationContext queryCompilationContext) - => new SqliteQueryTranslationPreprocessor(Dependencies, RelationalDependencies, queryCompilationContext); -} diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs index f0eec83f85d..8b1268f397a 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.Data.Sqlite; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.Sqlite.Internal; using Microsoft.EntityFrameworkCore.Sqlite.Storage.Internal; @@ -17,6 +18,7 @@ public class SqliteQueryableMethodTranslatingExpressionVisitor : RelationalQuery { private readonly IRelationalTypeMappingSource _typeMappingSource; private readonly ISqlExpressionFactory _sqlExpressionFactory; + private readonly bool _areJsonFunctionsSupported; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -32,6 +34,8 @@ public SqliteQueryableMethodTranslatingExpressionVisitor( { _typeMappingSource = relationalDependencies.TypeMappingSource; _sqlExpressionFactory = relationalDependencies.SqlExpressionFactory; + + _areJsonFunctionsSupported = new Version(new SqliteConnection().ServerVersion) >= new Version(3, 38); } /// @@ -46,6 +50,8 @@ protected SqliteQueryableMethodTranslatingExpressionVisitor( { _typeMappingSource = parentVisitor._typeMappingSource; _sqlExpressionFactory = parentVisitor._sqlExpressionFactory; + + _areJsonFunctionsSupported = parentVisitor._areJsonFunctionsSupported; } /// @@ -192,11 +198,20 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis /// 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 ShapedQueryExpression TranslateCollection( + protected override ShapedQueryExpression? TranslateCollection( SqlExpression sqlExpression, RelationalTypeMapping? elementTypeMapping, string tableAlias) { + // Support for JSON functions (e.g. json_each) was added in Sqlite 3.38.0 (2022-02-22, see https://www.sqlite.org/json1.html). + // This determines whether we have json_each, which is needed to query into JSON columns. + if (!_areJsonFunctionsSupported) + { + AddTranslationErrorDetails(SqliteStrings.QueryingIntoJsonCollectionsNotSupported(new SqliteConnection().ServerVersion)); + + return null; + } + var elementClrType = sqlExpression.Type.GetSequenceType(); var jsonEachExpression = new TableValuedFunctionExpression(tableAlias, "json_each", new[] { sqlExpression }); diff --git a/src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs b/src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs index e3cebd332b1..436d19b0b55 100644 --- a/src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs +++ b/src/EFCore.Sqlite.Core/Storage/Internal/SqliteTypeMappingSource.cs @@ -95,8 +95,6 @@ private static readonly HashSet SpatialiteTypes { TextTypeName, Text } }; - private readonly bool _areJsonFunctionsSupported; - /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -108,9 +106,6 @@ public SqliteTypeMappingSource( RelationalTypeMappingSourceDependencies relationalDependencies) : base(dependencies, relationalDependencies) { - // Support for JSON functions was added in Sqlite 3.38.0 (2022-02-22, see https://www.sqlite.org/json1.html). - // This determines whether we have json_each, which is needed to query into JSON columns. - _areJsonFunctionsSupported = new Version(new SqliteConnection().ServerVersion) >= new Version(3, 38); } /// @@ -210,28 +205,22 @@ public static bool IsSpatialiteType(string columnType) stringTypeMapping = (SqliteStringTypeMapping)stringTypeMapping .Clone(new CollectionToJsonStringConverter(mappingInfo.ClrType, elementTypeMapping)); - // json_each was introduced in SQLite 3.38.0; on older SQLite version we allow mapping the column, but don't set the element - // type mapping on the mapping, so that it isn't queryable. This causes us to go into the old translation path for Contains - // over parameter via IN with constants. - if (_areJsonFunctionsSupported) + switch (elementTypeMapping) { - switch (elementTypeMapping) - { - // The JSON representation for DateTimeOffset is ISO8601 (2023-01-01T12:30:00+02:00), but our SQL literal representation - // is 2023-01-01 12:30:00+02:00 (no T). - // datetime('2023-01-01T12:30:00+02:00') yields '2023-01-01 10:30:00' - converted to UTC, no timezone. - case SqliteDateTimeOffsetTypeMapping: - // The JSON representation for decimal is e.g. 1 (JSON int), whereas our literal representation is "1.0" (string) - case SqliteDecimalTypeMapping: - // The JSON representation for new[] { 1, 2 } is AQI= (base64?), our SQL literal representation is X'0102' - case ByteArrayTypeMapping: - break; - - - default: - stringTypeMapping = (SqliteStringTypeMapping)stringTypeMapping.CloneWithElementTypeMapping(elementTypeMapping); - break; - } + // The JSON representation for DateTimeOffset is ISO8601 (2023-01-01T12:30:00+02:00), but our SQL literal representation + // is 2023-01-01 12:30:00+02:00 (no T). + // datetime('2023-01-01T12:30:00+02:00') yields '2023-01-01 10:30:00' - converted to UTC, no timezone. + case SqliteDateTimeOffsetTypeMapping: + // The JSON representation for decimal is e.g. 1 (JSON int), whereas our literal representation is "1.0" (string) + case SqliteDecimalTypeMapping: + // The JSON representation for new[] { 1, 2 } is AQI= (base64?), our SQL literal representation is X'0102' + case ByteArrayTypeMapping: + break; + + + default: + stringTypeMapping = (SqliteStringTypeMapping)stringTypeMapping.CloneWithElementTypeMapping(elementTypeMapping); + break; } return stringTypeMapping; diff --git a/src/EFCore/Query/QueryTranslationPreprocessor.cs b/src/EFCore/Query/QueryTranslationPreprocessor.cs index 5fd863e6581..620278f0bb6 100644 --- a/src/EFCore/Query/QueryTranslationPreprocessor.cs +++ b/src/EFCore/Query/QueryTranslationPreprocessor.cs @@ -90,5 +90,5 @@ public virtual Expression NormalizeQueryableMethod(Expression expression) /// The query expression to process. /// A query expression after query roots have been added. protected virtual Expression ProcessQueryRoots(Expression expression) - => expression; + => new QueryRootProcessor(Dependencies, QueryCompilationContext).Visit(expression); } diff --git a/src/EFCore/Query/QueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore/Query/QueryableMethodTranslatingExpressionVisitor.cs index 7d4ab4a455e..b226b40b6dc 100644 --- a/src/EFCore/Query/QueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore/Query/QueryableMethodTranslatingExpressionVisitor.cs @@ -45,6 +45,8 @@ protected QueryableMethodTranslatingExpressionVisitor( /// protected virtual QueryableMethodTranslatingExpressionVisitorDependencies Dependencies { get; } + private Expression? _untranslatedExpression; + /// /// Detailed information about errors encountered during translation. /// @@ -56,7 +58,29 @@ protected QueryableMethodTranslatingExpressionVisitor( /// An expression to translate. /// A SQL translation of the given expression. public virtual Expression Translate(Expression expression) - => Visit(expression); + { + var translated = Visit(expression); + + // Note that we only throw if a specific node is recognized as untranslatable; we need to otherwise not throw in order to allow + // for client evaluation. + if (translated == QueryCompilationContext.NotTranslatedExpression && _untranslatedExpression is not null) + { + if (_untranslatedExpression is QueryRootExpression) + { + throw new InvalidOperationException( + TranslationErrorDetails is null + ? CoreStrings.QueryUnhandledQueryRootExpression(_untranslatedExpression.GetType().ShortDisplayName()) + : CoreStrings.TranslationFailedWithDetails(_untranslatedExpression, TranslationErrorDetails)); + } + + throw new InvalidOperationException( + TranslationErrorDetails is null + ? CoreStrings.TranslationFailed(_untranslatedExpression.Print()) + : CoreStrings.TranslationFailedWithDetails(_untranslatedExpression.Print(), TranslationErrorDetails)); + } + + return translated; + } /// /// Adds detailed information about errors encountered during translation. @@ -84,18 +108,15 @@ protected override Expression VisitExtension(Expression extensionExpression) { if (extensionExpression is QueryRootExpression queryRootExpression) { - // Query roots must be processed. - if (extensionExpression.GetType() == typeof(EntityQueryRootExpression) - && extensionExpression is EntityQueryRootExpression entityQueryRootExpression) + // This requires exact type match on query root to avoid processing query roots derived from EntityQueryRootExpression, e.g. + // SQL Server TemporalQueryRootExpression. + if (queryRootExpression.GetType() == typeof(EntityQueryRootExpression)) { - // This requires exact type match on query root to avoid processing derived query roots. - return CreateShapedQueryExpression(entityQueryRootExpression.EntityType); + return CreateShapedQueryExpression(((EntityQueryRootExpression)extensionExpression).EntityType); } - throw new InvalidOperationException( - TranslationErrorDetails is null - ? CoreStrings.QueryUnhandledQueryRootExpression(queryRootExpression.GetType().ShortDisplayName()) - : CoreStrings.TranslationFailedWithDetails(queryRootExpression, TranslationErrorDetails)); + _untranslatedExpression = queryRootExpression; + return QueryCompilationContext.NotTranslatedExpression; } return base.VisitExtension(extensionExpression); @@ -104,15 +125,6 @@ TranslationErrorDetails is null /// protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression) { - ShapedQueryExpression CheckTranslated(ShapedQueryExpression? translated) - => translated - ?? throw new InvalidOperationException( - TranslationErrorDetails == null - ? CoreStrings.TranslationFailed(methodCallExpression.Print()) - : CoreStrings.TranslationFailedWithDetails( - methodCallExpression.Print(), - TranslationErrorDetails)); - var method = methodCallExpression.Method; if (method.DeclaringType == typeof(Queryable) || method.DeclaringType == typeof(QueryableExtensions)) @@ -485,8 +497,24 @@ when QueryableMethods.IsSumWithSelector(method): LambdaExpression GetLambdaExpressionFromArgument(int argumentIndex) => methodCallExpression.Arguments[argumentIndex].UnwrapLambdaFromQuote(); + + Expression CheckTranslated(ShapedQueryExpression? translated) + { + if (translated is not null) + { + return translated; + } + + _untranslatedExpression ??= methodCallExpression; + + return QueryCompilationContext.NotTranslatedExpression; + } } } + else if (source == QueryCompilationContext.NotTranslatedExpression) + { + return source; + } } return _subquery diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/PrimitiveCollectionsQueryOldSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/PrimitiveCollectionsQueryOldSqlServerTest.cs index 8c9675236a4..4c335a6e0b0 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/PrimitiveCollectionsQueryOldSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/PrimitiveCollectionsQueryOldSqlServerTest.cs @@ -1,6 +1,8 @@ // 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.SqlServer.Internal; + namespace Microsoft.EntityFrameworkCore.Query; /// @@ -213,11 +215,11 @@ WHERE [p].[Id] NOT IN (2, 999) } public override Task Parameter_collection_Count(bool async) - => AssertTranslationFailed(() => base.Parameter_collection_Count(async)); + => AssertCompatibilityLevelTooLow(() => base.Parameter_collection_Count(async)); public override async Task Parameter_collection_of_ints_Contains(bool async) { - await base.Parameter_collection_of_nullable_ints_Contains_int(async); + await base.Parameter_collection_of_ints_Contains(async); AssertSql( """ @@ -312,19 +314,19 @@ FROM [PrimitiveCollectionsEntity] AS [p] } public override Task Column_collection_of_ints_Contains(bool async) - => AssertTranslationFailed(() => base.Column_collection_of_ints_Contains(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_of_ints_Contains(async)); public override Task Column_collection_of_nullable_ints_Contains(bool async) - => AssertTranslationFailed(() => base.Column_collection_of_nullable_ints_Contains(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_of_nullable_ints_Contains(async)); public override Task Column_collection_of_nullable_ints_Contains_null(bool async) - => AssertTranslationFailed(() => base.Column_collection_of_nullable_ints_Contains_null(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_of_nullable_ints_Contains_null(async)); public override Task Column_collection_of_strings_contains_null(bool async) => AssertTranslationFailed(() => base.Column_collection_of_strings_contains_null(async)); public override Task Column_collection_of_bools_Contains(bool async) - => AssertTranslationFailed(() => base.Column_collection_of_bools_Contains(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_of_bools_Contains(async)); [ConditionalFact] public virtual async Task Json_representation_of_bool_array() @@ -337,22 +339,22 @@ public virtual async Task Json_representation_of_bool_array() } public override Task Column_collection_Count_method(bool async) - => AssertTranslationFailed(() => base.Column_collection_Count_method(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_Count_method(async)); public override Task Column_collection_Length(bool async) - => AssertTranslationFailed(() => base.Column_collection_Length(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_Length(async)); public override Task Column_collection_index_int(bool async) - => AssertTranslationFailed(() => base.Column_collection_index_int(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_index_int(async)); public override Task Column_collection_index_string(bool async) - => AssertTranslationFailed(() => base.Column_collection_index_string(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_index_string(async)); public override Task Column_collection_index_datetime(bool async) - => AssertTranslationFailed(() => base.Column_collection_index_datetime(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_index_datetime(async)); public override Task Column_collection_index_beyond_end(bool async) - => AssertTranslationFailed(() => base.Column_collection_index_beyond_end(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_index_beyond_end(async)); public override async Task Inline_collection_index_Column(bool async) { @@ -371,28 +373,28 @@ ORDER BY [v].[_ord] } public override Task Parameter_collection_index_Column_equal_Column(bool async) - => AssertTranslationFailed(() => base.Parameter_collection_index_Column_equal_Column(async)); + => AssertCompatibilityLevelTooLow(() => base.Parameter_collection_index_Column_equal_Column(async)); public override Task Parameter_collection_index_Column_equal_constant(bool async) - => AssertTranslationFailed(() => base.Parameter_collection_index_Column_equal_constant(async)); + => AssertCompatibilityLevelTooLow(() => base.Parameter_collection_index_Column_equal_constant(async)); public override Task Column_collection_ElementAt(bool async) - => AssertTranslationFailed(() => base.Column_collection_ElementAt(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_ElementAt(async)); public override Task Column_collection_Skip(bool async) - => AssertTranslationFailed(() => base.Column_collection_Skip(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_Skip(async)); public override Task Column_collection_Take(bool async) - => AssertTranslationFailed(() => base.Column_collection_Take(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_Take(async)); public override Task Column_collection_Skip_Take(bool async) - => AssertTranslationFailed(() => base.Column_collection_Skip_Take(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_Skip_Take(async)); public override Task Column_collection_OrderByDescending_ElementAt(bool async) => AssertTranslationFailed(() => base.Column_collection_OrderByDescending_ElementAt(async)); public override Task Column_collection_Any(bool async) - => AssertTranslationFailed(() => base.Column_collection_Any(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_Any(async)); public override async Task Column_collection_projection_from_top_level(bool async) { @@ -410,19 +412,19 @@ public override Task Column_collection_Join_parameter_collection(bool async) => AssertTranslationFailed(() => base.Column_collection_Join_parameter_collection(async)); public override Task Inline_collection_Join_ordered_column_collection(bool async) - => AssertTranslationFailed(() => base.Inline_collection_Join_ordered_column_collection(async)); + => AssertCompatibilityLevelTooLow(() => base.Inline_collection_Join_ordered_column_collection(async)); public override Task Parameter_collection_Concat_column_collection(bool async) - => AssertTranslationFailed(() => base.Parameter_collection_Concat_column_collection(async)); + => AssertCompatibilityLevelTooLow(() => base.Parameter_collection_Concat_column_collection(async)); public override Task Column_collection_Union_parameter_collection(bool async) - => AssertTranslationFailed(() => base.Column_collection_Union_parameter_collection(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_Union_parameter_collection(async)); public override Task Column_collection_Intersect_inline_collection(bool async) - => AssertTranslationFailed(() => base.Column_collection_Intersect_inline_collection(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_Intersect_inline_collection(async)); public override Task Inline_collection_Except_column_collection(bool async) - => AssertTranslationFailed(() => base.Inline_collection_Except_column_collection(async)); + => AssertCompatibilityLevelTooLow(() => base.Inline_collection_Except_column_collection(async)); public override async Task Column_collection_equality_parameter_collection(bool async) { @@ -465,7 +467,7 @@ public override async Task Column_collection_equality_inline_collection_with_par } public override Task Parameter_collection_in_subquery_Union_column_collection_as_compiled_query(bool async) - => AssertTranslationFailed(() => base.Parameter_collection_in_subquery_Union_column_collection_as_compiled_query(async)); + => AssertCompatibilityLevelTooLow(() => base.Parameter_collection_in_subquery_Union_column_collection_as_compiled_query(async)); public override void Parameter_collection_in_subquery_and_Convert_as_compiled_query() { @@ -476,12 +478,15 @@ public override Task Parameter_collection_in_subquery_Count_as_compiled_query(bo => AssertTranslationFailed(() => base.Parameter_collection_in_subquery_Count_as_compiled_query(async)); public override Task Column_collection_in_subquery_Union_parameter_collection(bool async) - => AssertTranslationFailed(() => base.Column_collection_in_subquery_Union_parameter_collection(async)); + => AssertCompatibilityLevelTooLow(() => base.Column_collection_in_subquery_Union_parameter_collection(async)); [ConditionalFact] public virtual void Check_all_tests_overridden() => TestHelpers.AssertAllMethodsOverridden(GetType()); + private Task AssertCompatibilityLevelTooLow(Func query) + => AssertTranslationFailedWithDetails(query, SqlServerStrings.CompatibilityLevelTooLowForScalarCollections(120)); + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); From 62bf50aed2b29b1cb62402679e2be811b6682a1b Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 11 Jul 2023 10:10:22 +0200 Subject: [PATCH 2/2] Change jsonpath expressions compatibility level message --- .../Properties/SqlServerStrings.Designer.cs | 8 ++- .../Properties/SqlServerStrings.resx | 56 +++++++++---------- .../Internal/SqlServerQuerySqlGenerator.cs | 14 ++--- 3 files changed, 40 insertions(+), 38 deletions(-) diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index f82784ab5d3..1020f2d326a 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -200,10 +200,12 @@ public static string InvalidTableToIncludeInScaffolding(object? table) table); /// - /// A non-constant array index or property name was used when navigating inside a JSON document; this is only supported starting with SQL Server 2017. + /// A non-constant array index or property name was used when navigating inside a JSON document, but EF Core's SQL Server compatibility level is set to {compatibilityLevel}; this is only supported with compatibility level 140 (SQL Server 2017) or higher. /// - public static string JsonValuePathExpressionsNotSupported - => GetString("JsonValuePathExpressionsNotSupported"); + public static string JsonValuePathExpressionsNotSupported(object? compatibilityLevel) + => string.Format( + GetString("JsonValuePathExpressionsNotSupported", nameof(compatibilityLevel)), + compatibilityLevel); /// /// The properties {properties} are configured to use 'Identity' value generation and are mapped to the same table '{table}', but only one column per table can be configured as 'Identity'. Call 'ValueGeneratedNever' in 'OnModelCreating' for properties that should not use 'Identity'. diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index 114cd64c8fb..cf56d4bda6f 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -1,17 +1,17 @@  - @@ -187,7 +187,7 @@ The specified table '{table}' is not in a valid format. Specify tables using the format '[schema].[table]'. - A non-constant array index or property name was used when navigating inside a JSON document; this is only supported starting with SQL Server 2017. + A non-constant array index or property name was used when navigating inside a JSON document, but EF Core's SQL Server compatibility level is set to {compatibilityLevel}; this is only supported with compatibility level 140 (SQL Server 2017) or higher. The property '{property}' on entity type '{entityType}' is of type 'byte', but is set up to use a SQL Server identity column; this requires that values starting at 255 and counting down will be used for temporary key values. A temporary key value is needed for every entity inserted in a single call to 'SaveChanges'. Care must be taken that these values do not collide with real key values. diff --git a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs index 8a32db8ccda..22bf1c34c2f 100644 --- a/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs +++ b/src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs @@ -19,7 +19,7 @@ public class SqlServerQuerySqlGenerator : QuerySqlGenerator { private readonly IRelationalTypeMappingSource _typeMappingSource; private readonly ISqlGenerationHelper _sqlGenerationHelper; - private readonly bool _supportsJsonValueExpressions; + private readonly int _sqlServerCompatibilityLevel; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -35,10 +35,7 @@ public SqlServerQuerySqlGenerator( { _typeMappingSource = typeMappingSource; _sqlGenerationHelper = dependencies.SqlGenerationHelper; - - // JSON functions such as JSON_VALUE only support arbitrary expressions for the path parameter in SQL Server 2017 and above; before - // that, arguments must be constant strings. - _supportsJsonValueExpressions = sqlServerSingletonOptions.CompatibilityLevel >= 140; + _sqlServerCompatibilityLevel = sqlServerSingletonOptions.CompatibilityLevel; } /// @@ -433,11 +430,13 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp case { ArrayIndex: SqlExpression arrayIndex }: Sql.Append("["); + // JSON functions such as JSON_VALUE only support arbitrary expressions for the path parameter in SQL Server 2017 and + // above; before that, arguments must be constant strings. if (arrayIndex is SqlConstantExpression) { Visit(pathSegment.ArrayIndex); } - else if (_supportsJsonValueExpressions) + else if (_sqlServerCompatibilityLevel >= 140) { Sql.Append("' + CAST("); Visit(arrayIndex); @@ -447,7 +446,8 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp } else { - throw new InvalidOperationException(SqlServerStrings.JsonValuePathExpressionsNotSupported); + throw new InvalidOperationException( + SqlServerStrings.JsonValuePathExpressionsNotSupported(_sqlServerCompatibilityLevel)); } Sql.Append("]");