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

[8.0-rc.1] 'dbcontext optimize' does not preserve convertsNulls #31942

Closed
schuettecarsten opened this issue Oct 3, 2023 · 8 comments · Fixed by #31967
Closed

[8.0-rc.1] 'dbcontext optimize' does not preserve convertsNulls #31942

schuettecarsten opened this issue Oct 3, 2023 · 8 comments · Fixed by #31967
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Milestone

Comments

@schuettecarsten
Copy link

schuettecarsten commented Oct 3, 2023

I upgraded my code from 7.0.11 to 8.0.0-rc.1 just to test compatibility. When creating an pre-compiled model using dotnet tool run dotnet-ef -- dbcontext optimize ...`, I get the following exception. It works when using 7.0.11 libraries.

System.NotSupportedException: Encountered a constant of unsupported type '<>c__DisplayClass2_0'. Only primitive constant nodes are supported.
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.<VisitConstant>g__GenerateValue|36_0(Object value, <>c__DisplayClass36_0&)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitConstant(ConstantExpression constant)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Translate[T](Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitMember(MemberExpression member)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Translate[T](Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateList(IReadOnlyList`1 list)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateMethodArguments(ParameterInfo[] parameters, IReadOnlyList`1 arguments)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitMethodCall(MethodCallExpression call)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.Translate(Expression node)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.VisitLambda[T](Expression`1 lambda)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateCore(Expression node, ISet`1 collectedNamespaces, Boolean statementContext)
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqToCSharpSyntaxTranslator.TranslateExpression(Expression node, ISet`1 collectedNamespaces)
   at Microsoft.EntityFrameworkCore.Design.Internal.CSharpHelper.Expression(Expression node, ISet`1 collectedNamespaces)
   at Microsoft.EntityFrameworkCore.Design.Internal.CSharpRuntimeAnnotationCodeGenerator.Create(ValueConverter converter, CSharpRuntimeAnnotationCodeGeneratorParameters parameters, ICSharpHelper codeHelper)
   at Microsoft.EntityFrameworkCore.Design.Internal.RelationalCSharpRuntimeAnnotationCodeGenerator.Create(CoreTypeMapping typeMapping, CSharpRuntimeAnnotationCodeGeneratorParameters parameters, ValueComparer valueComparer, ValueComparer keyValueComparer, ValueComparer providerValueComparer)
   at Microsoft.EntityFrameworkCore.Design.Internal.ICSharpRuntimeAnnotationCodeGenerator.Create(CoreTypeMapping typeMapping, IProperty property, CSharpRuntimeAnnotationCodeGeneratorParameters parameters)
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.Create(IProperty property, String variableName, Dictionary`2 propertyVariables, CSharpRuntimeAnnotationCodeGeneratorParameters parameters)
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.Create(IProperty property, Dictionary`2 propertyVariables, CSharpRuntimeAnnotationCodeGeneratorParameters parameters)
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.CreateEntityType(IEntityType entityType, IndentedStringBuilder mainBuilder, IndentedStringBuilder methodBuilder, SortedSet`1 namespaces, String className, Boolean nullable)
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.GenerateEntityType(IEntityType entityType, String namespace, String className, Boolean nullable)
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGenerator.GenerateModel(IModel model, CompiledModelCodeGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Scaffolding.Internal.CompiledModelScaffolder.ScaffoldModel(IModel model, String outputDir, CompiledModelCodeGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Design.Internal.DbContextOperations.Optimize(String outputDir, String modelNamespace, String contextTypeName)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OptimizeContextImpl(String outputDir, String modelNamespace, String contextType)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OptimizeContext.<>c__DisplayClass0_0.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
Encountered a constant of unsupported type '<>c__DisplayClass2_0'. Only primitive constant nodes are supported.

I use -v option, but as no more details are shown after build completes, I cannot analyze what exactly goes wrong.

@ajcvickers
Copy link
Member

@schuettecarsten Please try with the latest daily build and if you still see an issue then please attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

@schuettecarsten
Copy link
Author

schuettecarsten commented Oct 4, 2023

@ajcvickers I tried latest build (8.0.0-rtm.23503.6) and get the same error. The error seems to be caused by ValueConverter, I use several classes that are implemented like the example here: https://learn.microsoft.com/en-us/ef/core/modeling/value-conversions?tabs=data-annotations#pre-defined-conversions

Update: Root cause found. I have a custom ValueConverter that has this constructor:

	/// <summary>
	/// Initializes a new instance of the <see cref="CompressedBsonValueConverter{T}"/> class.
	/// </summary>
	public CompressedBsonValueConverter(CompressionLevel level, ConverterMappingHints? hints = default)
		: base(
			v => DoSerialize(v, level),
			v => DoDeserialize(v),
			hints)
	{
	}

This worked fine in v7 and now fails when creating the model code. I had to remove the level parameter from the DoSerialize call to make it work again. Unfortunately, the level value is lost. So, how to pass additional values to ValueConverters in v8?

@schuettecarsten
Copy link
Author

schuettecarsten commented Oct 4, 2023

@ajcvickers I also found that convertsNulls value is lost when the model code is generated.

	/// <summary>
	/// Initializes a new instance of the <see cref="CompressedBsonValueConverter{T}"/> class.
	/// </summary>
	public CompressedBsonValueConverter(CompressionLevel level, ConverterMappingHints? hints = default)
		: base(
			v => DoSerialize(v),
			v => DoDeserialize(v),
			false, // <-- convertsNulls
			hints)

The generated code is:

                converter: new ValueConverter<object, byte[]>(
                    (object v) => CompressedBsonValueConverter<object>.DoSerializeSmallestSize(v),
                    (Byte[] v) => CompressedBsonValueConverter<object>.DoDeserialize(v)),
[...]
                jsonValueReaderWriter: new JsonConvertedValueReaderWriter<object, byte[]>(
                    JsonByteArrayReaderWriter.Instance,
                    new ValueConverter<object, byte[]>(
                        (object v) => CompressedBsonValueConverter<object>.DoSerializeSmallestSize(v),
                        (Byte[] v) => CompressedBsonValueConverter<object>.DoDeserialize(v))));

The same code is generated if I change the convertsNulls parameter to true.

@schuettecarsten
Copy link
Author

schuettecarsten commented Oct 4, 2023

@ajcvickers Is is by design / expected that the code generator creates lots of code that seems to be unneccessary? Here is one example, the following code is generated for every SQL server column. This creates dozends of identical ValueComparer instances, lots of expressions and redundant compiled lambda functions....

            tenantID.TypeMapping = SqlServerLongTypeMapping.Default.Clone(
                comparer: new ValueComparer<long>(
                    (long v1, long v2) => v1 == v2,
                    (long v) => v.GetHashCode(),
                    (long v) => v),
                keyComparer: new ValueComparer<long>(
                    (long v1, long v2) => v1 == v2,
                    (long v) => v.GetHashCode(),
                    (long v) => v),
                providerValueComparer: new ValueComparer<long>(
                    (long v1, long v2) => v1 == v2,
                    (long v) => v.GetHashCode(),
                    (long v) => v));

For nullable columns, the code is more complex, but also always the same for each column of that type:

            defaultRecipientID.TypeMapping = SqlServerLongTypeMapping.Default.Clone(
                comparer: new ValueComparer<long?>(
                    (Nullable<long> v1, Nullable<long> v2) => v1.HasValue && v2.HasValue && (long)v1 == (long)v2 || !v1.HasValue && !v2.HasValue,
                    (Nullable<long> v) => v.HasValue ? ((long)v).GetHashCode() : 0,
                    (Nullable<long> v) => v.HasValue ? (Nullable<long>)(long)v : default(Nullable<long>)),
                keyComparer: new ValueComparer<long?>(
                    (Nullable<long> v1, Nullable<long> v2) => v1.HasValue && v2.HasValue && (long)v1 == (long)v2 || !v1.HasValue && !v2.HasValue,
                    (Nullable<long> v) => v.HasValue ? ((long)v).GetHashCode() : 0,
                    (Nullable<long> v) => v.HasValue ? (Nullable<long>)(long)v : default(Nullable<long>)),
                providerValueComparer: new ValueComparer<long?>(
                    (Nullable<long> v1, Nullable<long> v2) => v1.HasValue && v2.HasValue && (long)v1 == (long)v2 || !v1.HasValue && !v2.HasValue,
                    (Nullable<long> v) => v.HasValue ? ((long)v).GetHashCode() : 0,
                    (Nullable<long> v) => v.HasValue ? (Nullable<long>)(long)v : default(Nullable<long>)));

The same for SqlServerStringTypeMapping, SqlServerBoolTypeMapping, SqlServerByteArrayTypeMapping, GuidTypeMapping, ...

@ajcvickers
Copy link
Member

/cc @AndriySvyryd

@ajcvickers ajcvickers changed the title [8.0-rc.1] 'dbcontext optimize' fails with 'Only primitive constant nodes are supported.', works in 7.0.11 [8.0-rc.1] 'dbcontext optimize' does not preserve convertsNulls Oct 4, 2023
ajcvickers added a commit that referenced this issue Oct 4, 2023
Fixes #31942

### Description

Updates to the compiled model resulted in losing the `convertsNulls` state of a value converter.

### Customer impact

Silent regression from 7 resulting in different data returned.

### How found

Customer reported on daily build.

### Regression

Yes.

### Testing

Added tests.

### Risk

Low.
@ajcvickers
Copy link
Member

Root cause found. I have a custom ValueConverter that has this constructor:

This looks like the same case as #31859.

@AndriySvyryd AndriySvyryd added this to the 8.0.0 milestone Oct 4, 2023
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 4, 2023
@schuettecarsten
Copy link
Author

schuettecarsten commented Oct 4, 2023

@ajcvickers @AndriySvyryd
Thank you for the quick fix. So the generated code and the complexity for the TypeMapping stuff is by design?

@AndriySvyryd
Copy link
Member

@ajcvickers @AndriySvyryd Thank you for the quick fix. So the generated code and the complexity for the TypeMapping stuff is by design?

Yes, this is part of our future Native AOT support #29754
Meanwhile it will further decrease the startup time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants