From f53fe59cee2de88724dc2702b0460d77851deef4 Mon Sep 17 00:00:00 2001 From: maumar Date: Fri, 24 Jul 2020 10:39:55 -0700 Subject: [PATCH] Fix to #19609 - Query: allow user functions to be annotated with nullability propagation information Added fluent API for function to specify it's nullability and function parameter to specify whether it propagates null. Also added property to DbFunction attribute to allow specify nullability there. --- .../DbFunctionAttribute.cs | 20 ++++- .../Metadata/Builders/DbFunctionBuilder.cs | 15 +++- .../Builders/DbFunctionBuilderBase.cs | 2 +- .../Builders/DbFunctionParameterBuilder.cs | 12 +++ .../Builders/IConventionDbFunctionBuilder.cs | 21 ++++- ...RelationalDbFunctionAttributeConvention.cs | 5 ++ .../Metadata/IConventionDbFunction.cs | 16 +++- src/EFCore.Relational/Metadata/IDbFunction.cs | 5 ++ .../Metadata/IMutableDbFunction.cs | 7 +- .../Metadata/Internal/DbFunction.cs | 48 ++++++++++ .../Metadata/Internal/DbFunctionParameter.cs | 52 +++++++++++ .../Internal/InternalDbFunctionBuilder.cs | 37 ++++++++ .../InternalDbFunctionParameterBuilder.cs | 30 +++++++ .../Metadata/Internal/StoreFunction.cs | 9 ++ .../Properties/RelationalStrings.Designer.cs | 6 ++ .../Properties/RelationalStrings.resx | 3 + .../RelationalMethodCallTranslatorProvider.cs | 11 ++- .../SqlExpressions/SqlFunctionExpression.cs | 7 +- .../Query/UdfDbFunctionTestBase.cs | 89 +++++++++++++++++-- .../Query/UdfDbFunctionSqlServerTests.cs | 80 ++++++++++++++++- 20 files changed, 454 insertions(+), 21 deletions(-) diff --git a/src/EFCore.Abstractions/DbFunctionAttribute.cs b/src/EFCore.Abstractions/DbFunctionAttribute.cs index e913a6c4c98..a64cbff5ff4 100644 --- a/src/EFCore.Abstractions/DbFunctionAttribute.cs +++ b/src/EFCore.Abstractions/DbFunctionAttribute.cs @@ -17,9 +17,12 @@ namespace Microsoft.EntityFrameworkCore public class DbFunctionAttribute : Attribute #pragma warning restore CA1813 // Avoid unsealed attributes { + private static readonly bool DefaultNullable = true; + private string _name; private string _schema; private bool _builtIn; + private bool? _nullable; /// /// Initializes a new instance of the class. @@ -68,12 +71,27 @@ public virtual string Schema } /// - /// The value indicating wheather the database function is built-in or not. + /// The value indicating whether the database function is built-in or not. /// public virtual bool IsBuiltIn { get => _builtIn; set => _builtIn = value; } + + /// + /// The value indicating whether the database function can return null result or not. + /// + public virtual bool IsNullable + { + get => _nullable ?? DefaultNullable; + set => _nullable = value; + } + + /// + /// Use this method if you want to know the nullability of + /// the database function or if it was not specified. + /// + public bool? GetIsNullable() => _nullable; } } diff --git a/src/EFCore.Relational/Metadata/Builders/DbFunctionBuilder.cs b/src/EFCore.Relational/Metadata/Builders/DbFunctionBuilder.cs index 1c078f2728f..05ec03d9d07 100644 --- a/src/EFCore.Relational/Metadata/Builders/DbFunctionBuilder.cs +++ b/src/EFCore.Relational/Metadata/Builders/DbFunctionBuilder.cs @@ -45,11 +45,24 @@ public DbFunctionBuilder([NotNull] IMutableDbFunction function) /// /// Marks whether the database function is built-in. /// - /// The value indicating wheather the database function is built-in. + /// The value indicating whether the database function is built-in. /// The same builder instance so that multiple configuration calls can be chained. public new virtual DbFunctionBuilder IsBuiltIn(bool builtIn = true) => (DbFunctionBuilder)base.IsBuiltIn(builtIn); + + /// + /// Marks whether the database function can return null value. + /// + /// The value indicating whether the database function can return null. + /// The same builder instance so that multiple configuration calls can be chained. + public virtual DbFunctionBuilderBase IsNullable(bool nullable = true) + { + Builder.IsNullable(nullable, ConfigurationSource.Explicit); + + return this; + } + /// /// Sets the return store type of the database function. /// diff --git a/src/EFCore.Relational/Metadata/Builders/DbFunctionBuilderBase.cs b/src/EFCore.Relational/Metadata/Builders/DbFunctionBuilderBase.cs index dbcc61d70b1..3d0244021de 100644 --- a/src/EFCore.Relational/Metadata/Builders/DbFunctionBuilderBase.cs +++ b/src/EFCore.Relational/Metadata/Builders/DbFunctionBuilderBase.cs @@ -77,7 +77,7 @@ public virtual DbFunctionBuilderBase HasSchema([CanBeNull] string schema) /// /// Marks whether the database function is built-in. /// - /// The value indicating wheather the database function is built-in. + /// The value indicating whether the database function is built-in. /// The same builder instance so that multiple configuration calls can be chained. public virtual DbFunctionBuilderBase IsBuiltIn(bool builtIn = true) { diff --git a/src/EFCore.Relational/Metadata/Builders/DbFunctionParameterBuilder.cs b/src/EFCore.Relational/Metadata/Builders/DbFunctionParameterBuilder.cs index dbf94a36903..3b0ddabe1a8 100644 --- a/src/EFCore.Relational/Metadata/Builders/DbFunctionParameterBuilder.cs +++ b/src/EFCore.Relational/Metadata/Builders/DbFunctionParameterBuilder.cs @@ -62,6 +62,18 @@ public virtual DbFunctionParameterBuilder HasStoreType([CanBeNull] string storeT return this; } + /// + /// Indicates whether parameter propagates nullability, meaning if it's value is null the database function itself returns null. + /// + /// Value which indicates whether parameter propagates nullability. + /// The same builder instance so that further configuration calls can be chained. + public virtual DbFunctionParameterBuilder PropagatesNullability(bool propagatesNullability = true) + { + Builder.PropagatesNullability(propagatesNullability, ConfigurationSource.Explicit); + + return this; + } + #region Hidden System.Object members /// diff --git a/src/EFCore.Relational/Metadata/Builders/IConventionDbFunctionBuilder.cs b/src/EFCore.Relational/Metadata/Builders/IConventionDbFunctionBuilder.cs index 0d586eebd6d..ff297a31eee 100644 --- a/src/EFCore.Relational/Metadata/Builders/IConventionDbFunctionBuilder.cs +++ b/src/EFCore.Relational/Metadata/Builders/IConventionDbFunctionBuilder.cs @@ -58,7 +58,7 @@ public interface IConventionDbFunctionBuilder : IConventionAnnotatableBuilder bool CanSetSchema([CanBeNull] string schema, bool fromDataAnnotation = false); /// - /// Sets the value indicating wheather the database function is built-in or not. + /// Sets the value indicating whether the database function is built-in or not. /// /// The value indicating whether the database function is built-in or not. /// Indicates whether the configuration was specified using a data annotation. @@ -76,6 +76,25 @@ public interface IConventionDbFunctionBuilder : IConventionAnnotatableBuilder /// if the given schema can be set for the database function. bool CanSetIsBuiltIn(bool builtIn, bool fromDataAnnotation = false); + /// + /// Sets the value indicating whether the database function can return null value or not. + /// + /// The value indicating whether the database function is built-in or not. + /// Indicates whether the configuration was specified using a data annotation. + /// + /// The same builder instance if the configuration was applied, + /// otherwise. + /// + IConventionDbFunctionBuilder IsNullable(bool nullable, bool fromDataAnnotation = false); + + /// + /// Returns a value indicating whether the given nullable can be set for the database function. + /// + /// The value indicating whether the database function can return null value or not. + /// Indicates whether the configuration was specified using a data annotation. + /// if the given schema can be set for the database function. + bool CanSetIsNullable(bool nullable, bool fromDataAnnotation = false); + /// /// Sets the store type of the function in the database. /// diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalDbFunctionAttributeConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalDbFunctionAttributeConvention.cs index b4b4ab5205b..07a8ff93796 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalDbFunctionAttributeConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalDbFunctionAttributeConvention.cs @@ -92,6 +92,11 @@ protected virtual void ProcessDbFunctionAdded( { dbFunctionBuilder.IsBuiltIn(dbFunctionAttribute.IsBuiltIn, fromDataAnnotation: true); } + + if (!dbFunctionAttribute.GetIsNullable() != null) + { + dbFunctionBuilder.IsNullable(dbFunctionAttribute.IsNullable, fromDataAnnotation: true); + } } } } diff --git a/src/EFCore.Relational/Metadata/IConventionDbFunction.cs b/src/EFCore.Relational/Metadata/IConventionDbFunction.cs index c14b125607a..2a02dd4fb9d 100644 --- a/src/EFCore.Relational/Metadata/IConventionDbFunction.cs +++ b/src/EFCore.Relational/Metadata/IConventionDbFunction.cs @@ -61,7 +61,7 @@ public interface IConventionDbFunction : IConventionAnnotatable, IDbFunction ConfigurationSource? GetSchemaConfigurationSource(); /// - /// Sets the value indicating wheather the database function is built-in or not. + /// Sets the value indicating whether the database function is built-in or not. /// /// The value indicating whether the database function is built-in or not. /// Indicates whether the configuration was specified using a data annotation. @@ -74,6 +74,20 @@ public interface IConventionDbFunction : IConventionAnnotatable, IDbFunction /// The configuration source for . ConfigurationSource? GetIsBuiltInConfigurationSource(); + /// + /// Sets the value indicating whether the database function can return null value or not. + /// + /// The value indicating whether the database function can return null value or not. + /// Indicates whether the configuration was specified using a data annotation. + /// The configured value. + bool SetIsNullable(bool nullable, bool fromDataAnnotation = false); + + /// + /// Gets the configuration source for . + /// + /// The configuration source for . + ConfigurationSource? GetIsNullableConfigurationSource(); + /// /// Sets the store type of the function in the database. /// diff --git a/src/EFCore.Relational/Metadata/IDbFunction.cs b/src/EFCore.Relational/Metadata/IDbFunction.cs index 0e56baf139e..c015b9677ab 100644 --- a/src/EFCore.Relational/Metadata/IDbFunction.cs +++ b/src/EFCore.Relational/Metadata/IDbFunction.cs @@ -55,6 +55,11 @@ public interface IDbFunction : IAnnotatable /// bool IsAggregate { get; } + /// + /// Gets the value indicating whether the database function can return null. + /// + bool IsNullable { get; } + /// /// Gets the configured store type string. /// diff --git a/src/EFCore.Relational/Metadata/IMutableDbFunction.cs b/src/EFCore.Relational/Metadata/IMutableDbFunction.cs index dc2e2ea5f09..8b4e6b96938 100644 --- a/src/EFCore.Relational/Metadata/IMutableDbFunction.cs +++ b/src/EFCore.Relational/Metadata/IMutableDbFunction.cs @@ -26,10 +26,15 @@ public interface IMutableDbFunction : IMutableAnnotatable, IDbFunction new string Schema { get; [param: CanBeNull] set; } /// - /// Gets or sets the value indicating wheather the database function is built-in or not. + /// Gets or sets the value indicating whether the database function is built-in or not. /// new bool IsBuiltIn { get; set; } + /// + /// Gets or sets the value indicating whether the database function can return null value or not. + /// + new bool IsNullable { get; set; } + /// /// Gets or sets the store type of the function in the database. /// diff --git a/src/EFCore.Relational/Metadata/Internal/DbFunction.cs b/src/EFCore.Relational/Metadata/Internal/DbFunction.cs index a44b4f46019..fc18071abd5 100644 --- a/src/EFCore.Relational/Metadata/Internal/DbFunction.cs +++ b/src/EFCore.Relational/Metadata/Internal/DbFunction.cs @@ -30,6 +30,7 @@ public class DbFunction : ConventionAnnotatable, IMutableDbFunction, IConvention private string _schema; private string _name; private bool _builtIn; + private bool _nullable; private string _storeType; private RelationalTypeMapping _typeMapping; private Func, SqlExpression> _translation; @@ -38,6 +39,7 @@ public class DbFunction : ConventionAnnotatable, IMutableDbFunction, IConvention private ConfigurationSource? _schemaConfigurationSource; private ConfigurationSource? _nameConfigurationSource; private ConfigurationSource? _builtInConfigurationSource; + private ConfigurationSource? _nullableConfigurationSource; private ConfigurationSource? _storeTypeConfigurationSource; private ConfigurationSource? _typeMappingConfigurationSource; private ConfigurationSource? _translationConfigurationSource; @@ -113,6 +115,8 @@ public DbFunction( : parameters .Select(p => new DbFunctionParameter(this, p.Name, p.Type)) .ToList(); + + _nullable = true; } private static string GetFunctionName(MethodInfo methodInfo, ParameterInfo[] parameters) @@ -386,6 +390,45 @@ public virtual bool SetIsBuiltIn(bool builtIn, ConfigurationSource configuration /// public virtual ConfigurationSource? GetIsBuiltInConfigurationSource() => _builtInConfigurationSource; + /// + /// 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 bool IsNullable + { + get => _nullable; + set => SetIsNullable(value, ConfigurationSource.Explicit); + } + + /// + /// 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 bool SetIsNullable(bool nullable, ConfigurationSource configurationSource) + { + if (!IsScalar) + { + new InvalidOperationException(RelationalStrings.NullabilityInfoOnlyAllowedOnScalarFunctions); + } + + _nullable = nullable; + _nullableConfigurationSource = configurationSource.Max(_nullableConfigurationSource); + + return nullable; + } + + /// + /// 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 ConfigurationSource? GetIsNullableConfigurationSource() => _nullableConfigurationSource; + /// /// 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 @@ -605,6 +648,11 @@ string IConventionDbFunction.SetSchema(string schema, bool fromDataAnnotation) bool IConventionDbFunction.SetIsBuiltIn(bool builtIn, bool fromDataAnnotation) => SetIsBuiltIn(builtIn, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// + [DebuggerStepThrough] + bool IConventionDbFunction.SetIsNullable(bool nullable, bool fromDataAnnotation) + => SetIsNullable(nullable, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// [DebuggerStepThrough] string IConventionDbFunction.SetStoreType(string storeType, bool fromDataAnnotation) diff --git a/src/EFCore.Relational/Metadata/Internal/DbFunctionParameter.cs b/src/EFCore.Relational/Metadata/Internal/DbFunctionParameter.cs index f159443271d..6d586025ee0 100644 --- a/src/EFCore.Relational/Metadata/Internal/DbFunctionParameter.cs +++ b/src/EFCore.Relational/Metadata/Internal/DbFunctionParameter.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Builders.Internal; @@ -22,9 +23,11 @@ public class DbFunctionParameter : ConventionAnnotatable, IMutableDbFunctionPara { private string _storeType; private RelationalTypeMapping _typeMapping; + private bool _propagatesNullability; private ConfigurationSource? _storeTypeConfigurationSource; private ConfigurationSource? _typeMappingConfigurationSource; + private ConfigurationSource? _propagatesNullabilityConfigurationSource; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -177,6 +180,55 @@ public virtual RelationalTypeMapping SetTypeMapping( private void UpdateTypeMappingConfigurationSource(ConfigurationSource configurationSource) => _typeMappingConfigurationSource = configurationSource.Max(_typeMappingConfigurationSource); + /// + /// 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 bool PropagatesNullability + { + get => _propagatesNullability; + set => SetPropagatesNullability(value, ConfigurationSource.Explicit); + } + + /// + /// 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 bool SetPropagatesNullability(bool propagatesNullability, ConfigurationSource configurationSource) + { + if (!Function.IsScalar) + { + new InvalidOperationException(RelationalStrings.NullabilityInfoOnlyAllowedOnScalarFunctions); + } + + _propagatesNullability = propagatesNullability; + + UpdatePropagatesNullabilityConfigurationSource(configurationSource); + + return propagatesNullability; + } + + /// + /// 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. + /// + private void UpdatePropagatesNullabilityConfigurationSource(ConfigurationSource configurationSource) + => _propagatesNullabilityConfigurationSource = configurationSource.Max(_storeTypeConfigurationSource); + + /// + /// 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 ConfigurationSource? GetPropagatesNullabilityConfigurationSource() => _propagatesNullabilityConfigurationSource; + /// /// 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 diff --git a/src/EFCore.Relational/Metadata/Internal/InternalDbFunctionBuilder.cs b/src/EFCore.Relational/Metadata/Internal/InternalDbFunctionBuilder.cs index e22612d781b..ad536d64bfc 100644 --- a/src/EFCore.Relational/Metadata/Internal/InternalDbFunctionBuilder.cs +++ b/src/EFCore.Relational/Metadata/Internal/InternalDbFunctionBuilder.cs @@ -116,6 +116,33 @@ public virtual bool CanSetIsBuiltIn(bool builtIn, ConfigurationSource configurat => configurationSource.Overrides(Metadata.GetIsBuiltInConfigurationSource()) || Metadata.IsBuiltIn == builtIn; + /// + /// 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 IConventionDbFunctionBuilder IsNullable(bool nullable, ConfigurationSource configurationSource) + { + if (CanSetIsNullable(nullable, configurationSource)) + { + Metadata.SetIsNullable(nullable, configurationSource); + return this; + } + + return null; + } + + /// + /// 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 bool CanSetIsNullable(bool nullable, ConfigurationSource configurationSource) + => configurationSource.Overrides(Metadata.GetIsNullableConfigurationSource()) + || Metadata.IsNullable == nullable; + /// /// 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 @@ -255,6 +282,16 @@ IConventionDbFunctionBuilder IConventionDbFunctionBuilder.IsBuiltIn(bool builtIn bool IConventionDbFunctionBuilder.CanSetIsBuiltIn(bool builtIn, bool fromDataAnnotation) => CanSetIsBuiltIn(builtIn, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// + [DebuggerStepThrough] + IConventionDbFunctionBuilder IConventionDbFunctionBuilder.IsNullable(bool nullable, bool fromDataAnnotation) + => IsNullable(nullable, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + + /// + [DebuggerStepThrough] + bool IConventionDbFunctionBuilder.CanSetIsNullable(bool nullable, bool fromDataAnnotation) + => CanSetIsNullable(nullable, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + /// [DebuggerStepThrough] IConventionDbFunctionBuilder IConventionDbFunctionBuilder.HasStoreType(string storeType, bool fromDataAnnotation) diff --git a/src/EFCore.Relational/Metadata/Internal/InternalDbFunctionParameterBuilder.cs b/src/EFCore.Relational/Metadata/Internal/InternalDbFunctionParameterBuilder.cs index 7ece80be473..914cfc3b2cc 100644 --- a/src/EFCore.Relational/Metadata/Internal/InternalDbFunctionParameterBuilder.cs +++ b/src/EFCore.Relational/Metadata/Internal/InternalDbFunctionParameterBuilder.cs @@ -31,6 +31,7 @@ public InternalDbFunctionParameterBuilder([NotNull] DbFunctionParameter paramete : base(parameter, modelBuilder) { } + /// /// 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 @@ -87,6 +88,35 @@ public virtual bool CanSetTypeMapping([CanBeNull] RelationalTypeMapping typeMapp => configurationSource.Overrides(Metadata.GetTypeMappingConfigurationSource()) || Metadata.TypeMapping == typeMapping; + /// + /// 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 IConventionDbFunctionParameterBuilder PropagatesNullability( + bool propagatesNullability, + ConfigurationSource configurationSource) + { + if (CanSetPropagatesNullability(propagatesNullability, configurationSource)) + { + Metadata.SetPropagatesNullability(propagatesNullability, configurationSource); + return this; + } + + return null; + } + + /// + /// 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 bool CanSetPropagatesNullability(bool propagatesNullability, ConfigurationSource configurationSource) + => configurationSource.Overrides(Metadata.GetPropagatesNullabilityConfigurationSource()) + || Metadata.PropagatesNullability == propagatesNullability; + /// IConventionDbFunctionParameter IConventionDbFunctionParameterBuilder.Metadata { diff --git a/src/EFCore.Relational/Metadata/Internal/StoreFunction.cs b/src/EFCore.Relational/Metadata/Internal/StoreFunction.cs index e905df86fd4..e93bc6477f7 100644 --- a/src/EFCore.Relational/Metadata/Internal/StoreFunction.cs +++ b/src/EFCore.Relational/Metadata/Internal/StoreFunction.cs @@ -29,6 +29,7 @@ public StoreFunction([NotNull] DbFunction dbFunction, [NotNull] RelationalModel { DbFunctions = new SortedDictionary() { { dbFunction.ModelName, dbFunction } }; IsBuiltIn = dbFunction.IsBuiltIn; + IsNullable = dbFunction.IsNullable; ReturnType = dbFunction.StoreType; Parameters = new StoreFunctionParameter[dbFunction.Parameters.Count]; @@ -50,6 +51,14 @@ public StoreFunction([NotNull] DbFunction dbFunction, [NotNull] RelationalModel /// public virtual bool IsBuiltIn { 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 bool IsNullable { get; } + /// public virtual string ReturnType { get; } diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 40adae121bc..d9a5fc25ae8 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1026,6 +1026,12 @@ public static string TableValuedFunctionNonTPH([CanBeNull] object dbFunction, [C public static string CustomQueryMappingOnOwner => GetString("CustomQueryMappingOnOwner"); + /// + /// Nullability information should only be specified for scalar database functions. + /// + public static string NullabilityInfoOnlyAllowedOnScalarFunctions + => GetString("NullabilityInfoOnlyAllowedOnScalarFunctions"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 61c0ba4df3f..8f2e54bcdc5 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -729,4 +729,7 @@ Using 'FromSqlRaw' or 'FromSqlInterpolated' on an entity type which has owned reference navigations sharing same table is not supported. + + Nullability information should only be specified for scalar database functions. + \ No newline at end of file diff --git a/src/EFCore.Relational/Query/RelationalMethodCallTranslatorProvider.cs b/src/EFCore.Relational/Query/RelationalMethodCallTranslatorProvider.cs index 71262f8f956..02cfdcd0501 100644 --- a/src/EFCore.Relational/Query/RelationalMethodCallTranslatorProvider.cs +++ b/src/EFCore.Relational/Query/RelationalMethodCallTranslatorProvider.cs @@ -8,6 +8,7 @@ using System.Reflection; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Query.Internal; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.Utilities; @@ -82,8 +83,9 @@ public virtual SqlExpression Translate( return _sqlExpressionFactory.Function( dbFunction.Name, arguments, - nullable: true, - argumentsPropagateNullability: arguments.Select(a => false).ToList(), + nullable: dbFunction.IsNullable, + argumentsPropagateNullability: dbFunction.Parameters.Select(p => p is DbFunctionParameter fp + && fp.PropagatesNullability), method.ReturnType.UnwrapNullableType()); } @@ -91,8 +93,9 @@ public virtual SqlExpression Translate( dbFunction.Schema, dbFunction.Name, arguments, - nullable: true, - argumentsPropagateNullability: arguments.Select(a => false).ToList(), + nullable: dbFunction.IsNullable, + argumentsPropagateNullability: dbFunction.Parameters.Select(p => p is DbFunctionParameter fp + && fp.PropagatesNullability), method.ReturnType.UnwrapNullableType()); } diff --git a/src/EFCore.Relational/Query/SqlExpressions/SqlFunctionExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SqlFunctionExpression.cs index 8c61fd032a6..4d8ac7c1e94 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SqlFunctionExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SqlFunctionExpression.cs @@ -203,30 +203,35 @@ private SqlFunctionExpression( /// The name of the function. /// public virtual string Name { get; } + /// /// The schema in which the function is defined, if any. /// public virtual string Schema { get; } + /// /// A bool value indicating if the function is niladic. /// public virtual bool IsNiladic { get; } + /// /// A bool value indicating if the function is built-in. /// public virtual bool IsBuiltIn { get; } + /// /// The list of arguments of this function. /// public virtual IReadOnlyList Arguments { get; } + /// /// The instance on which this function is applied. /// public virtual SqlExpression Instance { get; } + /// /// A bool value indicating if the function can return null result. /// - public virtual bool IsNullable { get; private set; } /// diff --git a/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs index fa52a9a434a..e86aa012eaf 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs @@ -172,6 +172,15 @@ public enum ReportingPeriod [DbFunction(Schema = "dbo")] public static string IdentityString(string s) => throw new Exception(); + public static string IdentityStringPropagateNull(string s) => throw new Exception(); + + [DbFunction(IsNullable = false)] + public static string IdentityStringNonNullable(string s) => throw new Exception(); + + public static string IdentityStringNonNullableFluent(string s) => throw new Exception(); + + public string StringLength(string s) => throw new Exception(); + public int AddValues(int a, int b) { throw new NotImplementedException(); @@ -242,6 +251,12 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(AddValues), new[] { typeof(int), typeof(int) })); + modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(IdentityStringPropagateNull), new[] { typeof(string) })) + .HasParameter("s").PropagatesNullability(); + + modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(IdentityStringNonNullableFluent), new[] { typeof(string) })) + .IsNullable(false); + //Instance modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(CustomerOrderCountInstance))) .HasName("CustomerOrderCount"); @@ -264,6 +279,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) modelBuilder.Entity().ToTable("MultProductOrders").HasKey(mpo => mpo.OrderId); + modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(StringLength), new[] { typeof(string) })) + .HasParameter("s").PropagatesNullability(); + //Table modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(GetCustomerOrderCountByYear), new[] { typeof(int) })); modelBuilder.HasDbFunction(typeof(UDFSqlContext).GetMethod(nameof(GetCustomerOrderCountByYearOnlyFrom2000), new[] { typeof(int), typeof(bool) })); @@ -820,6 +838,61 @@ public virtual void Nullable_navigation_property_access_preserves_schema_for_sql Assert.Equal("Customer", result); } + [ConditionalFact] + public virtual void Compare_function_without_null_propagation_to_null() + { + using var context = CreateContext(); + + var result = context.Customers + .OrderBy(c => c.Id) + .Where(c => UDFSqlContext.IdentityString(c.FirstName) != null) + .ToList(); + + Assert.Equal(4, result.Count); + } + + [ConditionalFact] + public virtual void Compare_function_with_null_propagation_to_null() + { + using var context = CreateContext(); + + var result = context.Customers + .OrderBy(c => c.Id) + .Where(c => UDFSqlContext.IdentityStringPropagateNull(c.FirstName) != null) + .ToList(); + + Assert.Equal(4, result.Count); + } + + [ConditionalFact] + public virtual void Compare_non_nullable_function_to_null_gets_optimized() + { + using var context = CreateContext(); + + var result = context.Customers + .OrderBy(c => c.Id) + .Where(c => UDFSqlContext.IdentityStringNonNullable(c.FirstName) != null + && UDFSqlContext.IdentityStringNonNullableFluent(c.FirstName) != null) + .ToList(); + + Assert.Equal(4, result.Count); + } + + [ConditionalFact] + public virtual void Compare_functions_returning_int_that_take_nullable_param_which_propagates_null() + { + using var context = CreateContext(); + + //var prm = default(string); + + var result = context.Customers + .OrderBy(c => c.Id) + .Where(c => context.StringLength(c.FirstName) != context.StringLength(c.LastName)) + .ToList(); + + Assert.Equal(4, result.Count); + } + [ConditionalFact] public virtual void Scalar_Function_SqlFragment_Static() { @@ -1947,8 +2020,8 @@ public virtual void TVF_backing_entity_type_mapped_to_view() using (var context = CreateContext()) { var customers = (from t in context.Set() - orderby t.FirstName - select t).ToList(); + orderby t.FirstName + select t).ToList(); Assert.Equal(4, customers.Count); } @@ -1961,9 +2034,9 @@ public virtual void Udf_with_argument_being_comparison_to_null_parameter() { var prm = default(string); var query = (from c in context.Customers - from r in context.GetCustomerOrderCountByYearOnlyFrom2000(c.Id, c.LastName != prm) - orderby r.Year - select r + from r in context.GetCustomerOrderCountByYearOnlyFrom2000(c.Id, c.LastName != prm) + orderby r.Year + select r ).ToList(); Assert.Equal(4, query.Count); @@ -1996,9 +2069,9 @@ select r ClearLog(); var query = (from a in context.Addresses - from r in context.GetCustomerOrderCountByYearOnlyFrom2000(1, a.City == a.State) - orderby a.Id, r.Year - select r + from r in context.GetCustomerOrderCountByYearOnlyFrom2000(1, a.City == a.State) + orderby a.Id, r.Year + select r ).ToList(); Assert.Equal(expected.Count, query.Count); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs b/test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs index 0cdd309203c..a7736cafb89 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs @@ -219,6 +219,50 @@ FROM [Orders] AS [o] ORDER BY [o].[Id]"); } + public override void Compare_function_without_null_propagation_to_null() + { + base.Compare_function_without_null_propagation_to_null(); + + AssertSql( + @"SELECT [c].[Id], [c].[FirstName], [c].[LastName] +FROM [Customers] AS [c] +WHERE [dbo].[IdentityString]([c].[FirstName]) IS NOT NULL +ORDER BY [c].[Id]"); + } + + public override void Compare_function_with_null_propagation_to_null() + { + base.Compare_function_with_null_propagation_to_null(); + + AssertSql( + @"SELECT [c].[Id], [c].[FirstName], [c].[LastName] +FROM [Customers] AS [c] +WHERE [c].[FirstName] IS NOT NULL +ORDER BY [c].[Id]"); + } + + public override void Compare_non_nullable_function_to_null_gets_optimized() + { + base.Compare_non_nullable_function_to_null_gets_optimized(); + + AssertSql( + @"SELECT [c].[Id], [c].[FirstName], [c].[LastName] +FROM [Customers] AS [c] +ORDER BY [c].[Id]"); + } + + public override void Compare_functions_returning_int_that_take_nullable_param_which_propagates_null() + { + base.Compare_functions_returning_int_that_take_nullable_param_which_propagates_null(); + + AssertSql( + @"SELECT [c].[Id], [c].[FirstName], [c].[LastName] +FROM [Customers] AS [c] +WHERE (([dbo].[StringLength]([c].[FirstName]) <> [dbo].[StringLength]([c].[LastName])) OR ([c].[FirstName] IS NULL OR [c].[LastName] IS NULL)) AND ([c].[FirstName] IS NOT NULL OR [c].[LastName] IS NOT NULL) +ORDER BY [c].[Id]"); + } + + public override void Scalar_Function_SqlFragment_Static() { base.Scalar_Function_SqlFragment_Static(); @@ -890,11 +934,43 @@ returns bit end"); context.Database.ExecuteSqlRaw( - @"create function [dbo].[IdentityString] (@customerName nvarchar(max)) + @"create function [dbo].[IdentityString] (@s nvarchar(max)) + returns nvarchar(max) + as + begin + return @s; + end"); + + context.Database.ExecuteSqlRaw( + @"create function [dbo].[IdentityStringPropagatesNull] (@s nvarchar(max)) + returns nvarchar(max) + as + begin + return @s; + end"); + + context.Database.ExecuteSqlRaw( + @"create function [dbo].[IdentityStringNonNullable] (@s nvarchar(max)) + returns nvarchar(max) + as + begin + return COALESCE(@s, 'NULL'); + end"); + + context.Database.ExecuteSqlRaw( + @"create function [dbo].[IdentityStringNonNullableFluent] (@s nvarchar(max)) returns nvarchar(max) as begin - return @customerName; + return COALESCE(@s, 'NULL'); + end"); + + context.Database.ExecuteSqlRaw( + @"create function [dbo].[StringLength] (@s nvarchar(max)) + returns int + as + begin + return LEN(@s); end"); context.Database.ExecuteSqlRaw(