Underlying enum values are not mapped to the database enumeration for types registered into the npgsql type mapping

[dpsenner]

Still investigating into ways to interact with composite types in the postgresql database, we defined a composite that uses a database defined enum. While following this outline we added ModelBuilder.ForNpgsqlHasEnum invocations and further added a plugin that translates composite type members such that the composite type member is mapped in the query. It does so by implementing a custom Expression that basically does this:

// a class that extends Microsoft.EntityFrameworkCore.Query.ExpressionTranslators.IMemberTranslator
public Expression Translate(MemberExpression memberExpression)
{
	if (memberExpression.Member == ValueMember)
	{
		return new GetCompositeTypeMemberExpression(NpgsqlNameTranslator, memberExpression.Expression, memberExpression.Member);
	}

	if (memberExpression.Member == UnitMember)
	{
		return new GetCompositeTypeMemberExpression(NpgsqlNameTranslator, memberExpression.Expression, memberExpression.Member);
	}

	return null;
}

// a class that extends System.Linq.Expressions.Expression
protected override Expression Accept(ExpressionVisitor visitor)
{
	var leftBracket = new CustomUnaryExpression(Instance, "(", Instance.Type, false);
	var rightBracket = new CustomUnaryExpression(leftBracket, ")", Instance.Type, true);
	var memberExpression = new CustomUnaryExpression(rightBracket, $".{MemberName}", Type, true);
	return memberExpression;
}

This works exceptionally well and generates good queries that look alike:

2018-08-14 16:01:27.065 +02:00 [Error] [7856816] [14] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("11"ms) [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t"

Unfortunately when adding a simple where clause in the query as follows:

storage
	.WeighingRecords
	.Where(t => t.Weight.Unit == WeightUnit.Tonne)

the following happens:

2018-08-14 16:01:27.065 +02:00 [Error] [7856816] [14] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("11"ms) [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE (t.weight).unit = 6"
Npgsql.PostgresException (0x80004005): 42883: operator does not exist: units.weight_unit = integer

When I refactor the where argument out of the query such that it becomes a query parameter the query works as expected:

var weightUnit = WeightUnit.Tonne;
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Weight.Unit == weightUnit)

2018-08-14 16:13:28.360 +02:00 [Information] [8553120] [17] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Executed DbCommand ("10"ms) [Parameters=["@__weightUnit_0='Tonne' (DbType = Object)"], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE (t.weight).unit = @__weightUnit_0"

Is there a way to make this query work without that extra parameter?

[dpsenner]

I just noticed that changing this:

var weightUnit = WeightUnit.Tonne;
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Weight.Unit == weightUnit)

to a const variable as the roslynator code analyzer warns about RCS1118:

const WeightUnit weightUnit = WeightUnit.Tonne;
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Weight.Unit == weightUnit)

causes the query to fail again because the underlying int value of the enumeration is passed into the query as the parameter.

[austindrenski]

@dpsenner You're running into a few open issues here...

The constant enum issue is similar to the char literal issue. Basically the expression tree stores the underlying value directly (making this issue fairly intractable). There's been some discussion of how to tackle this, such as adding some heuristics to determine when a literal char or enum comparison is happening, but that work is still ongoing. (#450, #489)

As for composite mapping, there's some work that needs to happen at the ADO.NET driver before we can add first-class support to the EF Core provider. One recent consideration has been to transparently map composites to ValueTuple. (#563)

Based on your snippets, it looks like you're planning to replace some services internal to the EF Core provider. If you're already set on doing this, you should take a look at the separation of responsibilities between our custom expressions, the NpgsqlSqlTranslatingExpressionVisitor, and the NpgsqlQuerySqlGenerator.

Right now it looks like you're using CustomUnaryExpression to generate SQL for the composite types. While creative, this type of work is much better suited for a service replacing NpgsqlQuerySqlGenerator.

[dpsenner]

@austindrenski Thanks, #593 reads like a duplicate of #489. It seems like I misinterpreted the error because it read to me like being related to composite types. Are the changes of #490 up for the next release? If these changes should be part of 2.1.1.1 then #490 does not work for types mapped from a plugin.

The following is a little bit off topic to this issue, I do however feel that parts of what we are doing could become a part of the postgresql integration with EF core.

We're not considering to replace EF core services or are not seeing that we should do so. Currently the following snippets are part of an NpgsqlEntityFrameworkPlugin that adds database enums and composite types that we have defined in the database. It does so by adding RelationalTypeMappers and IMemberTranslators and is a spinoff from this conversation.

Feel free to have a look at the plugin. As you see below, we use the NpgsqlEntityFrameworkPlugin system to add support for our types and I assume that this is the initial purpose of this plugin. We have no need to replace ef core internal services. We are somehow abusing the relational type mapping system to inject non-relational composite types. It would be better if there was a way to add composite type mappings in the plugin, also because it would allow us to delegate the member translation of composite types to NpgsqlQuerySqlGenerator which appears to be the right place for this.

The following is the plugin:

public class UnitsPlugin : NpgsqlEntityFrameworkPlugin
{
	public override string Name => "Schema.Units";

	public override string Description => "Provides conversions for unit types and enumerations in the unit schema.";

	protected NpgsqlSnakeCaseNameTranslator NpgsqlSnakeCaseNameTranslator { get; }

	private Dictionary<Type, (RelationalTypeMapping, IMemberTranslator)> CompositeTypes { get; }

	public UnitsPlugin()
	{
		NpgsqlSnakeCaseNameTranslator = new NpgsqlSnakeCaseNameTranslator();
		CompositeTypes = new Dictionary<Type, (RelationalTypeMapping, IMemberTranslator)>();

		AddEnum<UnitKind>();
		AddEnum<AreaUnit>();
		AddCompositeType<Area>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<FrequencyUnit>();
		AddCompositeType<Frequency>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<LengthUnit>();
		AddCompositeType<Length>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<PieceCountUnit>();
		AddCompositeType<PieceCount>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<RatioUnit>();
		AddCompositeType<Ratio>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<TemperatureUnit>();
		AddCompositeType<Temperature>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<VolumeUnit>();
		AddCompositeType<Volume>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");

		AddEnum<WeightUnit>();
		AddCompositeType<Weight>(t => $"({ToInvariantString(t.Value)},{ToSnakeCase(t.Unit)})");
	}

	public override void AddMappings(NpgsqlTypeMappingSource typeMappingSource)
	{
		base.AddMappings(typeMappingSource);

		foreach ((Type compositeType, (RelationalTypeMapping relationalTypeMapping, IMemberTranslator memberTranslator)) in CompositeTypes.Select(t => (t.Key, t.Value)))
		{
			typeMappingSource.ClrTypeMappings[compositeType] = relationalTypeMapping;
			typeMappingSource.StoreTypeMappings[relationalTypeMapping.StoreType] = new RelationalTypeMapping[] { relationalTypeMapping };
		}
	}

	public override void AddMemberTranslators(NpgsqlCompositeMemberTranslator compositeMemberTranslator)
	{
		base.AddMemberTranslators(compositeMemberTranslator);

		IEnumerable<IMemberTranslator> memberTranslators = CompositeTypes.Select(t => t.Value.Item2);
		compositeMemberTranslator.AddTranslators(memberTranslators);
	}

	private void AddCompositeType<TComposite>(Func<TComposite, string> sqlLiteralFormattingFunc)
		where TComposite : class, new()
	{
		// add this composite type to the underlying npgsql type mapper
		string typeName = NpgsqlSnakeCaseNameTranslator.TranslateTypeName(typeof(TComposite).Name);
		string pgName = $"{Schema.Name}.{typeName}";
		NpgsqlConnection.GlobalTypeMapper.MapComposite<TComposite>(pgName);

		// add EF side type mapper and member translators for this composite type
		var sqlLiteralFormatter = new DefaultSqlLiteralFormatter<TComposite>(sqlLiteralFormattingFunc);
		RelationalTypeMapping relationalTypeMapping = GetRelationalTypeMapping(sqlLiteralFormatter);
		IMemberTranslator memberTranslator = GetMemberTranslator<TComposite>();
		CompositeTypes[typeof(TComposite)] = (relationalTypeMapping, memberTranslator);
	}

	private void AddEnum<TEnum>()
		where TEnum : struct
	{
		// add this enum to the underlying npgsql type mapper
		string enumName = NpgsqlSnakeCaseNameTranslator.TranslateTypeName(typeof(TEnum).Name);
		string pgName = $"{Schema.Name}.{enumName}";
		NpgsqlConnection.GlobalTypeMapper.MapEnum<TEnum>(pgName);
	}

	private RelationalTypeMapping GetRelationalTypeMapping<TComposite>(ISqlLiteralFormatter<TComposite> sqlLiteralFormatter)
	{
		const string schema = Schema.Name;
		string typeName = NpgsqlSnakeCaseNameTranslator.ConvertToSnakeCase(typeof(TComposite).Name);
		string storeType = $"{schema}.{typeName}";
		return new NpgsqlCompositeTypeMapping<TComposite>(storeType, sqlLiteralFormatter);
	}

	private IMemberTranslator GetMemberTranslator<TComposite>()
	{
		return new DefaultCompositeMemberTranslator<TComposite>(NpgsqlSnakeCaseNameTranslator);
	}

	private string ToInvariantString(long value)
	{
		return value.ToString(CultureInfo.InvariantCulture);
	}

	private string ToInvariantString(decimal value)
	{
		return value.ToString(CultureInfo.InvariantCulture);
	}

	private string ToSnakeCase<TUnit>(TUnit unit)
	{
		return NpgsqlSnakeCaseNameTranslator.TranslateMemberName($"{unit}");
	}
}

public class NpgsqlCompositeTypeMapping<TComposite> : NpgsqlTypeMapping
{
	protected ISqlLiteralFormatter<TComposite> SqlLiteralFormatter { get; }

	public NpgsqlCompositeTypeMapping(string storeType, ISqlLiteralFormatter<TComposite> sqlLiteralFormatter)
		: base(storeType, typeof(TComposite), NpgsqlTypes.NpgsqlDbType.Unknown)
	{
		SqlLiteralFormatter = sqlLiteralFormatter ?? throw new ArgumentNullException(nameof(sqlLiteralFormatter));
	}

	protected NpgsqlCompositeTypeMapping(RelationalTypeMappingParameters parameters, ISqlLiteralFormatter<TComposite> sqlLiteralFormatter)
		: base(parameters, NpgsqlTypes.NpgsqlDbType.Unknown)
	{
		SqlLiteralFormatter = sqlLiteralFormatter ?? throw new ArgumentNullException(nameof(sqlLiteralFormatter));
	}

	public override RelationalTypeMapping Clone(string storeType, int? size)
	{
		return new NpgsqlCompositeTypeMapping<TComposite>(Parameters.WithStoreTypeAndSize(storeType, size), SqlLiteralFormatter);
	}

	public override DbParameter CreateParameter(DbCommand command, string name, object value, bool? nullable = null)
	{
		string sqlLiteral = GetSqlLiteral(value);

		return base.CreateParameter(command, name, sqlLiteral, nullable);
	}

	protected override string GenerateNonNullSqlLiteral(object value)
	{
		return GetSqlLiteral(value);
	}

	private string GetSqlLiteral(object value)
	{
		if (!(value is TComposite composite))
		{
			throw new ArgumentException($"Expected an instance of type {typeof(TComposite)}", nameof(value));
		}

		return SqlLiteralFormatter.GetSqlLiteral(composite);
	}
}

public class DefaultCompositeMemberTranslator<TComposite> : IMemberTranslator
{
	protected Dictionary<MemberInfo, Func<MemberExpression, Expression>> MemberExpressionFactories { get; }

	public DefaultCompositeMemberTranslator(INpgsqlNameTranslator npgsqlNameTranslator)
	{
		MemberExpressionFactories = GetMemberExpressionFactories(npgsqlNameTranslator);
	}

	public Expression Translate(MemberExpression memberExpression)
	{
		MemberInfo member = memberExpression.Member;
		if (!MemberExpressionFactories.ContainsKey(member))
		{
			return null;
		}

		return MemberExpressionFactories[member](memberExpression);
	}

	private Dictionary<MemberInfo, Func<MemberExpression, Expression>> GetMemberExpressionFactories(INpgsqlNameTranslator npgsqlNameTranslator)
	{
		var result = new Dictionary<MemberInfo, Func<MemberExpression, Expression>>();
		foreach (var propertyMember in typeof(TComposite).GetProperties())
		{
			result[propertyMember] = (memberExpression) => new CompositeTypeMemberExpression(memberExpression.Expression, propertyMember.PropertyType, npgsqlNameTranslator.TranslateMemberName(propertyMember.Name));
		}

		return result;
	}
}

public class CompositeTypeMemberExpression : Expression
{
	public override ExpressionType NodeType => ExpressionType.Extension;

	public override Type Type { get; }

	protected Expression Instance { get; }

	protected string MemberName { get; }

	public CompositeTypeMemberExpression(Expression instance, Type memberType, string memberName)
	{
		Type = memberType;
		MemberName = memberName;

		Instance = instance ?? throw new ArgumentNullException(nameof(instance));
	}

	protected override Expression Accept(ExpressionVisitor visitor)
	{
		var leftBracket = new CustomUnaryExpression(Instance, "(", Instance.Type, false);
		var rightBracket = new CustomUnaryExpression(leftBracket, ")", Instance.Type, true);
		var memberExpression = new CustomUnaryExpression(rightBracket, $".{MemberName}", Type, true);
		return memberExpression;
	}

	protected override Expression VisitChildren(ExpressionVisitor visitor)
	{
		return visitor.Visit(Instance);
	}

	public override string ToString()
	{
		return $"({Instance}).{MemberName}";
	}
}

[dpsenner]

I just added an enumeration member to one of the models that maps to a database enum. The following is the script to create the database enum and table:

create schema sandbox;

create type sandbox.weighing_meaning AS ENUM (
	'meaning1',
	'meaning2'
);

create table sandbox.weighing (
	id uuid not null primary key default tooling.create_id(),
	weight units.weight not null,
	meaning sandbox.weighing_meaning not null
);

It appears like the query generator does indeed map the enum to the enum member:

var meaning = WeighingMeaning.Meaning2;
var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Meaning == meaning)
	.Select(t => new
	{
		t.Id,
		t.Weight.Value,
		t.Weight.Unit,
	})
	.ToList();

This is what is run against the database:

2018-08-16 12:53:29.780 +02:00 [Debug] [6522008] [16] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Executing DbCommand [Parameters=["@__meaning_0='Meaning2' (DbType = Object)"], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = @__meaning_0"

However, by refactoring the query to this:

var actualWeighingRecords = storage
	.WeighingRecords
	.Where(t => t.Meaning == WeighingMeaning.Meaning2)
	.Select(t => new
	{
		t.Id,
		t.Weight.Value,
		t.Weight.Unit,
	})
	.ToList();

the following happens:

2018-08-16 12:53:30.307 +02:00 [Error] [6901136] [16] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("12"ms) [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = 'meaning2'::\"sandbox.weighing_meaning\""
Npgsql.PostgresException (0x80004005): 42704: type "sandbox.weighing_meaning" does not exist
 at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
 at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext() in C:\projects\npgsql\src\Npgsql\NpgsqlConnector.cs:line 1032
--- End of stack trace from previous location where exception was thrown ---
 at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming) in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 444
 at Npgsql.NpgsqlDataReader.NextResult() in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 332
 at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1219
 at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1130
 at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)

The query is semantically correct but fails on the syntax because it added quotes around sandbox.weighing_meaning. The query without quotes works fine when run in pgadmin, i.e.:

SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = 'meaning2'::sandbox.weighing_meaning

It appears that somewhere the integral value is converted back to the enumeration and properly mapped to the correct enumeration member. The fix #490 addresses some issues, but there are still other usecases that do not work as one would expect.

[austindrenski]

@dpsenner I'll take another look at this tonight, but in the meantime take a look at #554 and let me know if you believe part of your issue overlaps.

Part of this is needing to differentiate between known enum issues, and issues related to composites + your plugin. Any help delineating the two would help immensely.

[dpsenner]

For one, enumerations that are not added before any dbcontext is created do not work as expected. It is mandatory to add enumerations to the underlying type mapper by invocation of NpgsqlConnection.GlobalTypeMapper.MapEnum<TEnum>(pgName) right here:

	.UseNpgsql(MrsCoreDatabase.ConnectionString, npgsqlOptions =>
	{
		// adds enumerations and composite types to the underlying npgsqlconnection global type mapper
		npgsqlOptions.AddBackendStorageTypes();
	})

If one does not do this, queries fail with the following symptom:

2018-08-16 17:42:32.790 +02:00 [Error] [6828096] [13] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("34"ms) [Parameters=["@p0='ea8937a0-d0b5-4d91-95e2-c573c0ee8917', @p1='0', @p2='(6,kilogram)' (Nullable = false) (DbType = Object)"], CommandType='Text', CommandTimeout='600']"
""INSERT INTO sandbox.weighing (id, meaning, weight)
VALUES (@p0, @p1, @p2);"
Npgsql.PostgresException (0x80004005): 42804: column "meaning" is of type sandbox.weighing_meaning but expression is of type integer

What does not work is adding enumeration types during the model builder phase. To mitigate the issue I just implemented the following extension method which does register npgsql enumeration members but requires the enumeration type to have been registered earlier:

public static ModelBuilder AddEnum<TEnum>(this ModelBuilder modelBuilder, string schema)
	where TEnum : struct
{
	Type enumType = typeof(TEnum);
	INpgsqlNameTranslator nameTranslator = new NpgsqlSnakeCaseNameTranslator();
	string name = nameTranslator.TranslateTypeName(enumType.Name);
	string[] labels = Enum.GetValues(enumType)
		.Cast<TEnum>()
		.Select(t => $"{t}")
		.Select(t => nameTranslator.TranslateMemberName(t))
		.ToArray();

	if (!NpgsqlConnection.GlobalTypeMapper.Mappings.Where(t => t.ClrTypes.Contains(enumType)).Any())
	{
		throw new InvalidOperationException($"The enumeration {enumType} is not registered with the npgsql global type mapper");
	}

	return modelBuilder.ForNpgsqlHasEnum(schema, name, labels);
}

This process feels terrible because there are several places and several times where types have to be registered but knowing this, one can make sure that it works. With enumerations added at the right time, queries fail again with this symptom:

2018-08-16 17:58:02.970 +02:00 [Debug] [7894608] [13] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Executing DbCommand [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = 'meaning2'::\"sandbox.weighing_meaning\""

[dpsenner]

Next I just tried to create the enumeration in the public schema:

create schema sandbox;
create type public.weighing_meaning AS ENUM (
	'meaning1',
	'meaning2'
);

create table sandbox.weighing (
	id uuid not null primary key default tooling.create_id(),
	weight units.weight not null,
	meaning public.weighing_meaning not null
);

Doing so and mapping the enumeration along with the schema appears to fail with the same symptom.

2018-08-16 17:58:02.970 +02:00 [Debug] [7894608] [13] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Executing DbCommand [Parameters=[""], CommandType='Text', CommandTimeout='600']"
""SELECT t.id, (t.weight).value, (t.weight).unit
FROM sandbox.weighing AS t
WHERE t.meaning = 'meaning2'::\"public.weighing_meaning\""

[dpsenner]

When I instead register the enum without a schema all my queries and tests pass. I.e. the following works:

NpgsqlConnection.GlobalTypeMapper.MapEnum<TEnum>("some_enum");

while the following fails:

NpgsqlConnection.GlobalTypeMapper.MapEnum<TEnum>("public.some_enum");

[austindrenski]

When I instead register the enum without a schema all my queries and tests pass

Good to hear. This is part of what #554 is looking to fix.

[dpsenner]

After further investigation I can confirm that we observe exactly two symptoms:

1. when an enumeration is a member of a relational model and the enumeration is defined in another schema, queries fail because the type is quoted; this is what Mapping issues with schema-scoped enums #554 is about
2. when an enumeration is a member of a composite type that is added to the global type mapper and not a relational model, the fix implemented with Enum downcasting throws on write #490 does not work for select queries as previously mentioned in this comment

To me it is still unclear whether the second symptom can and should be fixed by improving the plugin. The plugin currently does implement expressions to navigate into composite types members (via IMemberTranslator) and therefore feels like it could be the right place but at the same time it feels to be the wrong place for this. As outlined in a previous comment, it would be ideal if the NpgsqlEntityFrameworkPlugin class provided an api to inject composite types. NpgsqlEntityFrameworkPlugin could then set up composite type members' mappings to work both in select tuples and where queries.