From 50f41ad89d50f0b971afe87ad09537dff6d87852 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/IDbFunctionParameter.cs | 5 ++ .../Metadata/IMutableDbFunction.cs | 7 +- .../Metadata/Internal/DbFunction.cs | 51 +++++++++++ .../Metadata/Internal/DbFunctionParameter.cs | 57 +++++++++--- .../Internal/InternalDbFunctionBuilder.cs | 37 ++++++++ .../InternalDbFunctionParameterBuilder.cs | 30 +++++++ .../Properties/RelationalStrings.Designer.cs | 6 ++ .../Properties/RelationalStrings.resx | 3 + .../RelationalMethodCallTranslatorProvider.cs | 9 +- .../SqlExpressions/SqlFunctionExpression.cs | 7 +- .../Query/UdfDbFunctionTestBase.cs | 87 +++++++++++++++++-- .../Query/UdfDbFunctionSqlServerTests.cs | 80 ++++++++++++++++- 20 files changed, 440 insertions(+), 35 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..ebfba05b69c 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() is bool value) + { + dbFunctionBuilder.IsNullable(value, 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/IDbFunctionParameter.cs b/src/EFCore.Relational/Metadata/IDbFunctionParameter.cs index cfa2fcec41b..cc483f6c8d0 100644 --- a/src/EFCore.Relational/Metadata/IDbFunctionParameter.cs +++ b/src/EFCore.Relational/Metadata/IDbFunctionParameter.cs @@ -32,6 +32,11 @@ public interface IDbFunctionParameter : IAnnotatable /// string StoreType { get; } + /// + /// Gets the value which indicates whether parameter propagates nullability, meaning if it's value is null the database function itself returns null. + /// + bool PropagatesNullability { get; } + /// /// Gets the for this parameter. /// 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..1912799cea4 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,11 @@ public DbFunction( : parameters .Select(p => new DbFunctionParameter(this, p.Name, p.Type)) .ToList(); + + if (IsScalar) + { + _nullable = true; + } } private static string GetFunctionName(MethodInfo methodInfo, ParameterInfo[] parameters) @@ -386,6 +393,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 +651,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..2666e43bc7f 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 @@ -119,20 +122,11 @@ public virtual string SetStoreType([CanBeNull] string storeType, ConfigurationSo { _storeType = storeType; - UpdateStoreTypeConfigurationSource(configurationSource); + _storeTypeConfigurationSource = configurationSource.Max(_storeTypeConfigurationSource); return storeType; } - /// - /// 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 UpdateStoreTypeConfigurationSource(ConfigurationSource configurationSource) - => _storeTypeConfigurationSource = 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 @@ -168,14 +162,49 @@ public virtual RelationalTypeMapping SetTypeMapping( [NotNull] RelationalTypeMapping typeMapping, ConfigurationSource configurationSource) { _typeMapping = typeMapping; - - UpdateTypeMappingConfigurationSource(configurationSource); + _typeMappingConfigurationSource = configurationSource.Max(_typeMappingConfigurationSource); return typeMapping; } - 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; + _propagatesNullabilityConfigurationSource = configurationSource.Max(_storeTypeConfigurationSource); + + 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. + /// + public virtual ConfigurationSource? GetPropagatesNullabilityConfigurationSource() => _propagatesNullabilityConfigurationSource; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to 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/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..3a9db59ac1e 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,8 @@ 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.PropagatesNullability), method.ReturnType.UnwrapNullableType()); } @@ -91,8 +92,8 @@ 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.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..8928fc86b6a 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,59 @@ 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 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 +2018,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 +2032,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 +2067,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(