Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Json column fails to materialize only when default interceptor used #30244

Closed
gkelley-fmb opened this issue Feb 9, 2023 · 2 comments · Fixed by #30403
Closed

Json column fails to materialize only when default interceptor used #30244

gkelley-fmb opened this issue Feb 9, 2023 · 2 comments · Fixed by #30403
Assignees
Labels
area-interception area-json area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@gkelley-fmb
Copy link

I was trying to implement a materialization interceptor when I came across this issue. The attached project implements the interceptor but just as a pass through. The interceptor doesn't have to do anything to cause the error.

I do receive the same type of error as #29380 but I believe this is different as my Json doesn't use Id.

Attaching project that reproduces the issue. Run dotnet ef database update. Run application and watch it output the values in the json column. Uncomment line 14 in the TestDbContext and run the application again and get the following stack trace...

   at System.Linq.Expressions.Expression.ValidateLambdaArgs(Type delegateType, Expression& body, ReadOnlyCollection`1 parameters, String paramName)
   at System.Linq.Expressions.Expression.Lambda[TDelegate](Expression body, String name, Boolean tailCall, IEnumerable`1 parameters)
   at System.Linq.Expressions.Expression.Lambda[TDelegate](Expression body, Boolean tailCall, IEnumerable`1 parameters)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitArguments(ExpressionVisitor visitor, IArgumentProvider nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitNew(NewExpression node)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitArguments(ExpressionVisitor visitor, IArgumentProvider nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitMethodCall(MethodCallExpression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitBlockExpressions(ExpressionVisitor visitor, BlockExpression block)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitBinary(BinaryExpression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitBlockExpressions(ExpressionVisitor visitor, BlockExpression block)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitBlockExpressions(ExpressionVisitor visitor, BlockExpression block)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitSwitchCase(SwitchCase node)
   at System.Linq.Expressions.ExpressionVisitor.Visit[T](ReadOnlyCollection`1 nodes, Func`2 elementVisitor)
   at System.Linq.Expressions.ExpressionVisitor.VisitSwitch(SwitchExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitBinary(BinaryExpression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitBlockExpressions(ExpressionVisitor visitor, BlockExpression block)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitConditional(ConditionalExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitConditional(ConditionalExpression node)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitBlockExpressions(ExpressionVisitor visitor, BlockExpression block)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitBinary(BinaryExpression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.VisitBinary(BinaryExpression binaryExpression)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitBlockExpressions(ExpressionVisitor visitor, BlockExpression block)
   at System.Linq.Expressions.ExpressionVisitor.VisitBlock(BlockExpression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitLambda[T](Expression`1 node)
   at System.Dynamic.Utils.ExpressionVisitorUtils.VisitArguments(ExpressionVisitor visitor, IArgumentProvider nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitMethodCall(MethodCallExpression node)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ProcessShaper(Expression shaperExpression, RelationalCommandCache& relationalCommandCache, IReadOnlyList`1& readerColumns, LambdaExpression& relatedDataLoaders, Int32& collectionId)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitShapedQuery(ShapedQueryExpression shapedQueryExpression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   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 Program.<Main>$(String[] args) in D:\Source\repos\FMB.Net\IssueRepro\Program.cs:line 5

EF Core version: 7.0.2
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 7.0
Operating system: Windows 10 Pro
IDE: Visual Studio 2022 17.4.4
IssueRepro.zip

@maumar
Copy link
Contributor

maumar commented Mar 3, 2023

When building JSON entity using a materializer we generate the following materializer initially:

return 
{
	KeyValueSetting30244 instance;
	Dictionary<IPropertyBase, ValueTuple<object, Func<MaterializationContext, object>>> accessorDictionary;
	MaterializationInterceptionData materializationData;
	InterceptionResult<object> creatingResult;
	accessorDictionary = 
	{
		Dictionary<IPropertyBase, ValueTuple<object, Func<MaterializationContext, object>>> dictionary;
		dictionary = new Dictionary<IPropertyBase, ValueTuple<object, Func<MaterializationContext, object>>>();
		dictionary.Add(
			key: Property: KeyValueSetting30244.TestEntity30244Id (no field, int) Shadow Required PK FK AfterSave:Throw, 
			value: new ValueTuple<object, Func<MaterializationContext, object>>(
				materializationContext2 => ExpressionExtensions.ValueBufferTryReadValue<int>(
					valueBuffer: materializationContext2.get_ValueBuffer(), 
					index: 0, 
					property: Property: KeyValueSetting30244.TestEntity30244Id (no field, int) Shadow Required PK FK AfterSave:Throw), 
				materializationContext2 => (object)ExpressionExtensions.ValueBufferTryReadValue<int>(
					valueBuffer: materializationContext2.get_ValueBuffer(), 
					index: 0, 
					property: Property: KeyValueSetting30244.TestEntity30244Id (no field, int) Shadow Required PK FK AfterSave:Throw)
			));
                (...)
	}
}

when we then visit the materializer we replace ValueBufferTryReadValue. Normal JSON properties are replaced with calls to ShaperProcessingExpressionVisitor.ExtractJsonProperty<string> and the keys are replaced with access to keyValues array, which is object[].

So before we had expression returning int and now we have expression returning object. This happens as part of lambda expression rewrite and type change of the body is not allowed there.

@maumar
Copy link
Contributor

maumar commented Mar 3, 2023

this was not a problem before, because normally we store the key values in shadowValueBuffer which is typed as object[] as well (and so ValueBufferTryReadValue calls for key values are typed as object already), or (in case of NoTracking) we don't need to materialize the keys, since they are all in shadow state for JSON entities.

maumar added a commit that referenced this issue Mar 5, 2023
…terceptor used

Problem was that when we build materializer expression, ValueBufferTryReadValue call that extracts values of keys is typed with key's CLR type. This fails for json as we visit the materializer expression, because calls to ValueBufferTryReadValue are replaced with array accesses to object[] that we store key values in. We should be using ValueBufferTryReadValue<object> when trying to get value for keys, like we do everywhere else, so that after the visitation expression types are correct.

Fixes #30244
maumar added a commit that referenced this issue Mar 5, 2023
…terceptor used

Problem was that when we build materializer expression, ValueBufferTryReadValue call that extracts values of keys is typed with key's CLR type. This fails for json as we visit the materializer expression, because calls to ValueBufferTryReadValue are replaced with array accesses to object[] that we store key values in. We should be using ValueBufferTryReadValue<object> when trying to get value for keys, like we do everywhere else, so that after the visitation expression types are correct.

Fixes #30244
ajcvickers pushed a commit that referenced this issue Mar 15, 2023
…terceptor used

Problem was that when we build materializer expression, ValueBufferTryReadValue call that extracts values of keys is typed with key's CLR type. This fails for json as we visit the materializer expression, because calls to ValueBufferTryReadValue are replaced with array accesses to object[] that we store key values in. We should be using ValueBufferTryReadValue<object> when trying to get value for keys, like we do everywhere else, so that after the visitation expression types are correct.

Fixes #30244
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 15, 2023
maumar added a commit that referenced this issue Mar 15, 2023
…terceptor used (#30403)

* Fix to #30244 - Json column fails to materialize only when default interceptor used

Problem was that when we build materializer expression, ValueBufferTryReadValue call that extracts values of keys is typed with key's CLR type. This fails for json as we visit the materializer expression, because calls to ValueBufferTryReadValue are replaced with array accesses to object[] that we store key values in. We should be using ValueBufferTryReadValue<object> when trying to get value for keys, like we do everywhere else, so that after the visitation expression types are correct.

Fixes #30244

* Move testing to the interceptor tests, which are setup for singleton options
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview3 Mar 23, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview3, 8.0.0 Nov 14, 2023
@ajcvickers ajcvickers removed their assignment Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-interception area-json area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants