ArgumentNullException when comparing niladic functions

[bachratyg]

When using niladic functions query execution may throw an ArgumentNullException.

I think the bug is here: https://github.com/dotnet/efcore/blob/v5.0.3/src/EFCore.Relational/Query/SqlExpressions/SqlFunctionExpression.cs#L389 Null check missing for the Arguments property.

Seems like this was already fixed in #23296 for v6 but still an issue for v5.

Repro code

Project.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="5.0.3" />
  </ItemGroup>

</Project>

Program.cs - simplest

using System;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;

namespace ConsoleApp5
{
	class Program
	{
		static void Main()
		{
			var fn1 = new SqlFunctionExpression(functionName: "GETDATE", nullable: false, type: typeof(DateTime), typeMapping: null);
			var fn2 = new SqlFunctionExpression(functionName: "GETDATE", nullable: false, type: typeof(DateTime), typeMapping: null);
			var eq = fn1.Equals(fn2); // throws
		}
	}
}

Program.cs - using expression factory

			// Fix up deps manually
			var sql = new SqlExpressionFactory(new SqlExpressionFactoryDependencies(
				typeMappingSource: new SqlServerTypeMappingSource(
					dependencies: new TypeMappingSourceDependencies(
						valueConverterSelector: new ValueConverterSelector(new ValueConverterSelectorDependencies()),
						plugins: Array.Empty<ITypeMappingSourcePlugin>()
					),
					relationalDependencies: new RelationalTypeMappingSourceDependencies(Array.Empty<IRelationalTypeMappingSourcePlugin>())
				)
			));
			var fn1 = sql.NiladicFunction("GETDATE", false, typeof(DateTime));
			var fn2 = sql.NiladicFunction("GETDATE", false, typeof(DateTime));
			var eq = fn1.Equals(fn2); // throws
		}
	}
}

Exception thrown

Value cannot be null. (Parameter 'first')
   at System.Linq.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at System.Linq.Enumerable.SequenceEqual[TSource](IEnumerable`1 first, IEnumerable`1 second, IEqualityComparer`1 comparer)
   at System.Linq.Enumerable.SequenceEqual[TSource](IEnumerable`1 first, IEnumerable`1 second)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressions.SqlFunctionExpression.Equals(SqlFunctionExpression sqlFunctionExpression)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressions.SqlFunctionExpression.Equals(Object obj)

The actual production code where I'm hitting this is much more involved and for legal reasons I cannot disclose it, however the root cause seems to be the same: query execution triggers SqlFunctionExpression.Equals. Exception stack trace:

Value cannot be null. (Parameter 'first')
   at System.Linq.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at System.Linq.Enumerable.SequenceEqual[TSource](IEnumerable`1 first, IEnumerable`1 second, IEqualityComparer`1 comparer)
   at System.Linq.Enumerable.SequenceEqual[TSource](IEnumerable`1 first, IEnumerable`1 second)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressions.SqlFunctionExpression.Equals(SqlFunctionExpression sqlFunctionExpression)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressions.SqlFunctionExpression.Equals(Object obj)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressions.SelectExpression.<>c__DisplayClass57_0.<AddToProjection>b__0(ProjectionExpression pe)
   at System.Collections.Generic.List`1.FindIndex(Int32 startIndex, Int32 count, Predicate`1 match)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressions.SelectExpression.AddToProjection(SqlExpression sqlExpression, String alias)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressions.SelectExpression.ApplyProjection()
   at Microsoft.EntityFrameworkCore.Query.Internal.SelectExpressionProjectionApplyingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.SelectExpressionProjectionApplyingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryTranslationPostprocessor.Process(Expression query)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetEnumerator()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)

Provider and version information

EF Core version: 5.0.3
Database provider: Microsoft.EntityFrameworkCore.SqlServer - does not seem to matter, code path does not touch provider specifics
Target framework: .NETCore 3.1
Operating system: Windows 10 20H2 (19042.804)
IDE: Visual Studio 2019 16.8.5

[smitpatel]

Can you share a real LINQ query example where this is blocking?

[bachratyg]

This is a bit closer to a real-life query, runnable on any Oracle instance:
csproj

<PackageReference Include="Oracle.EntityFrameworkCore" Version="5.21.1" />

program

	[Table("DUAL")]
	class Dual
	{
		[Key]
		[Column("DUMMY")]
		public string Dummy { get; set; }
	}
	class AppDb : DbContext
	{
		public AppDb(DbContextOptions<AppDb> options) : base(options)
		{
		}
		public DbSet<Dual> Dual { get; set; }

		protected override void OnModelCreating(ModelBuilder modelBuilder)
		{
			modelBuilder.HasDbFunction(() => Fn.User()).HasTranslation(args => new SqlFunctionExpression("USER", false, typeof(string), null));
		}
	}
	static class Fn
	{
		public static string User() => throw new NotImplementedException("No client eval");
	}
	class Program
	{
		static void Main()
		{
			var services = new ServiceCollection();
			services.AddDbContext<AppDb>(opts =>
			{
				opts.UseOracle("...");
			});
			var svc = services.BuildServiceProvider();

			var db = svc.GetRequiredService<AppDb>();

			var q = from d in db.Dual
				select new
				{
					Creator = Fn.User(),
					Updater = Fn.User(),
				};

			var i = q.ToList();
		}
	}

Expected command text: SELECT USER "Creator", USER "Updater" FROM "DUAL" "d"
The exception happens when two projection members have the exact same expression or the left part of the expression tree is the same up to the point where the comparison hits the function expression e.g. this would also trigger the bug:

class MyTable
{
	[Key] public int Type { get; set; }
	public string Creator { get; set; }
	public string Updater { get; set; }
}
public DbSet<MyTable> MyTable { get; set; }
var q = from d in db.MyTable
	select new
	{
		Creator = d.Type == 1 ? Fn.User() : d.Creator,
		Updater = d.Type == 1 ? Fn.User() : d.Updater,
	};

but this does not:

var q = from d in db.MyTable
	select new
	{
		Creator = d.Type != 1 ? d.Creator : Fn.User(),
		Updater = d.Type != 1 ? d.Updater : Fn.User(),
	};

A simple workaround that comes to my mind:

.HasTranslation(args => new SqlFragmentExpression("USER"));

but now the command text is SELECT 1 FROM "DUAL" "d", and query execution hits the NotImplementedException in the function body. Am I missing something here?

Another possible workaround I found is a custom expression type like so:

.HasTranslation(args => new NiladicFunctionExpression("USER", false, typeof(string), null));
class NiladicFunctionExpression : SqlFunctionExpression
{
	public NiladicFunctionExpression(string functionName, bool nullable, Type type, RelationalTypeMapping typeMapping) : base(functionName, nullable, type, typeMapping)
	{
	}
	public override bool IsNiladic => true;
	public override IReadOnlyList<SqlExpression> Arguments => Array.Empty<SqlExpression>();
}

then I get SELECT USER "Creator" FROM "DUAL" "d" which is mostly fine. This would only break when a translator in the pipeline updates and reverts it to SqlFunctionExpression (unlikely).

Side note: the function here is deterministic but e.g. sys_guid() or rand() is not. Maybe the AddToProjection optimization (and the unconditional call to Equals) is a bit premature here? (different issue for a different day)

[ajcvickers]

@bachratyg See the planning process for information on how we decide which issues to patch. At the moment we're having trouble understanding the real impact of this on production code. Can you try to explain what is blocked in your production scenarios so that we can better articulate the impact when taking this to the directors for patch approval?

[bachratyg]

I managed to map all affected functions to behavior-equivalent non-niladic db calls (e.g. SYS_CONTEXT('USERENV', 'SESSION_USER') instead of USER). The provider/other 3rd party libs do not create niladic functions on their own so this should eliminate all instances from the app.

Closing as this is no longer a critical/blocking issue since there's a viable workaround and the bug seems already fixed in v6.

Note for posterity: as it turns out this is a regression in v5 from v3.1. The following translation works in v3.1:

...HasTranslation(_ => new SqlFunctionExpression(null, null, "USER", true, null, true, typeof(string), null));