Mistreating ENUMs whenever they are backed by something else than int

[ondrejtucny]

I am running into repeated issues whenever I work with enum fields which are backed by anything else but int.

This simple query:

// fetch all current CNP settings for the given cards
var projection =
    from cardSec in qe.All<Model.CardSecurity>()
    join cnp in qe.All<Model.CardCnpSetting>() on cardSec equals cnp.AsCurrent
    where QueryableExtensions.In<int>(cardSec.CardID, parentKeys)
        && cnp.State == HistoryTrackedItemState.Current
    select new { cardSec.CardID, cnp };

causes an exception:

Xtensive.Orm.QueryTranslationException: 'Unable to translate 'Query.All().Join(
  Query.All(),
  cardSec => cardSec,
  cnp => cnp.AsCurrent,
  (cardSec, cnp) => new @<cardSec, cnp>(
    cardSec,
    cnp
  )
).Where(<>h__TransparentIdentifier0 => (<>h__TransparentIdentifier0.cardSec.CardID.In(@.parentKeys) && (((Int32)<>h__TransparentIdentifier0.cnp.State) == 5))).Select(<>h__TransparentIdentifier0 => new @<CardID, cnp>(
  <>h__TransparentIdentifier0.cardSec.CardID,
  <>h__TransparentIdentifier0.cnp
))' expression. See inner exception for details.'

The inner exception is an "Ambiguous match found." error. And this is the stack trace:

   at System.RuntimeType.GetPropertyImpl(String name, BindingFlags bindingAttr, Binder binder, Type returnType, Type[] types, ParameterModifier[] modifiers)
   at System.Type.GetProperty(String name, BindingFlags bindingAttr)
   at Xtensive.Orm.Linq.Translator.VisitMemberAccess(MemberExpression ma)
   at Xtensive.Linq.ExpressionVisitor`1.Visit(Expression e)
   at Xtensive.Linq.ExpressionVisitor.VisitUnary(UnaryExpression u)
   at Xtensive.Orm.Linq.Translator.VisitUnary(UnaryExpression u)
   at Xtensive.Linq.ExpressionVisitor`1.Visit(Expression e)
   at Xtensive.Orm.Linq.Translator.VisitBinary(BinaryExpression binaryExpression)
   at Xtensive.Linq.ExpressionVisitor`1.Visit(Expression e)
   at Xtensive.Orm.Linq.Translator.VisitBinary(BinaryExpression binaryExpression)
   at Xtensive.Linq.ExpressionVisitor`1.Visit(Expression e)
   at Xtensive.Orm.Linq.Translator.VisitLambda(LambdaExpression le)
   at Xtensive.Orm.Linq.Translator.VisitWhere(Expression expression, LambdaExpression le)
   at Xtensive.Orm.Linq.Translator.VisitQueryableMethod(MethodCallExpression mc, QueryableMethodKind methodKind)
   at Xtensive.Linq.QueryableVisitor.VisitMethodCall(MethodCallExpression mc)
   at Xtensive.Orm.Linq.Translator.VisitMethodCall(MethodCallExpression mc)
   at Xtensive.Linq.ExpressionVisitor`1.Visit(Expression e)
   at Xtensive.Orm.Linq.Translator.VisitSequence(Expression sequenceExpression, Expression expressionPart)
   at Xtensive.Orm.Linq.Translator.VisitSelect(Expression expression, LambdaExpression le)
   at Xtensive.Orm.Linq.Translator.VisitQueryableMethod(MethodCallExpression mc, QueryableMethodKind methodKind)
   at Xtensive.Linq.QueryableVisitor.VisitMethodCall(MethodCallExpression mc)
   at Xtensive.Orm.Linq.Translator.VisitMethodCall(MethodCallExpression mc)
   at Xtensive.Linq.ExpressionVisitor`1.Visit(Expression e)
   at Xtensive.Orm.Linq.Translator.Translate()
   at Xtensive.Orm.Linq.QueryProvider.Translate(Expression expression, CompilerConfiguration compilerConfiguration)

I can try updating to the latest master, in a few days, to confirm this as a pending bug. However, I do believe this has a similar root cause as the mistreatment of non-int fields in general. I have seen DO generate casts in SQL queries, completely unnecessary, such as this: …AND ( CAST([a].[id_arch] AS integer) = 0));, where id_arch is a smallint column (short in C#).

It's a pain and I hope @alex-kulakov and the team will pay attention to this. I already know how to write unit tests in DO codebase, so I will try contributing at least tests to identify these issues.

[ondrejtucny]

Another example of an issue related to non-int fields:

Unable to cast object of type 'System.Int32' to type 'System.Int16'.

Happens when accessing an referenced entity, which has a short primary key:

   at System.Runtime.CompilerServices.CastHelpers.Unbox(Void* toTypeHnd, Object obj)
   at Microsoft.Data.SqlClient.SqlBuffer.get_Int16()
   at Xtensive.Sql.TypeMapper.ReadShort(DbDataReader reader, Int32 index)
   at Xtensive.Orm.Providers.DbDataReaderAccessor.Read(DbDataReader source)
   at Xtensive.Orm.Providers.BatchingCommandProcessor.ExecuteBatch(Int32 numberOfTasks, QueryRequest lastRequest, CommandProcessorContext context)
   at Xtensive.Orm.Providers.BatchingCommandProcessor.ExecuteTasks(CommandProcessorContext context)
   at Xtensive.Orm.Providers.SqlSessionHandler.ExecuteQueryTasks(IEnumerable`1 queryTasks, Boolean allowPartialExecution)
   at Xtensive.Orm.Session.ProcessInternalDelayedQueries(Boolean allowPartialExecution)
   at Xtensive.Orm.Internals.Prefetch.Fetcher.<ExecuteTasks>d__3.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Xtensive.Orm.Internals.Prefetch.PrefetchManager.<ExecuteTasks>d__20.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Xtensive.Orm.Providers.SqlSessionHandler.FetchEntityState(Key key)
   at Xtensive.Orm.QueryEndpoint.SingleOrDefault(Key key)
   at Xtensive.Orm.Internals.FieldAccessors.EntityFieldAccessor`1.GetValue(Persistent obj)
   at Xtensive.Orm.Persistent.GetFieldValue[T](FieldInfo field)

The data model in question uses mapping on a legacy database. Stripped down version of the entities is as follows:

[HierarchyRoot(InheritanceSchema.SingleTable)]
[TableMapping("d_product")]
public sealed class DProduct : LegacyUserTrackedEntity
{
    [Key]
    [Field]
    [FieldMapping("id_product")]
    public int Id { get; set; }

    [Association]
    [Field(Nullable = false)]
    [FieldMapping("id_fee_type")]
    public CFeeType FeeType { get; set; }
}

[HierarchyRoot(InheritanceSchema.SingleTable)]
[TableMapping("c_fee_type")]
public sealed class CFeeType : LegacyUserTrackedEntity
{
    [Key]
    [Field]
    [FieldMapping("id_fee_type")]
    public short Id { get; set; }

    [Field(Nullable = false, Length = 5)]
    [FieldMapping("code")]
    public string Code { get; set; }
}

The code which causes the exception, processes the result of a query, which returns records encapsulating related entities, and is as follows:

private record QueryProjection(…, DProduct Product, …);

private SomeDto Map(QueryProjection x)
{
    return new Dto()
    {
        …
        // TODO Retrieving the fee type code causes an exception in DO: cannot convert int to short
        FeeTypeCode = x.Product.FeeType.Code,
        …
    }
}

[ondrejtucny]

Update: The second issue was caused by a mismatched primary key data type (short instead of int) in an entity referenced from CFeeType. It is strange that during domain build and validation this is not recognized and not reported as a mismatch. Due to the implementation of integer value retrieval in the SQL client such check could save a lot of headaches.

See also: dotnet/runtime#19290 and source code here: https://github.com/dotnet/SqlClient/blob/73c6b2ff3ef3b55431cf187e72d5de918caccf76/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBuffer.cs#L409

[alex-kulakov]

Hello @ondrejtucny, thank you for the report, I'll look at these issues and be back with some investigation results or comments

[alex-kulakov]

By the way, cast to integer may be connected with the fact that literal parameters are by default integer, for example here … AND ( CAST([a].[id_arch] AS integer) = 0)) 0 is treated as integer value, I can only say for now is that code is generalized and probably affects values in different parts of statements. It is not important for comparisons to have left and right parts of the same type and a smallint value can be compared with an integer value but from expressions standpoint it is better be the same type.

[alex-kulakov]

As far as I know .NET does the same thing, it casts short to int. What is 0 in the example above? Is it constant? What type it was on on the level of IQueryable query?

[letarak]

Faced the same issue
Can be reproduced

short enum failed on sql server (ShortEnumType : short)

  Query.All<Entity>()
    .GroupBy(it => it.Structure == null 
             ? (ShortEnumType)(-1)
             :  it.Structure.EnumField)
    .Select(it => it.Key)
    .Take(10)
    .ToArray();

Unable to cast object of type 'System.Int32' to type 'System.Int16'.

  at System.Data.SqlClient.SqlBuffer.get_Int16()
   at Xtensive.Sql.TypeMapper.ReadShort(DbDataReader reader, Int32 index) in /_/Orm/Xtensive.Orm/Sql/ValueTypeMapping/TypeMapper.cs:line 198
   at Xtensive.Orm.Providers.DbDataReaderAccessor.Read(DbDataReader source) in /_/Orm/Xtensive.Orm/Orm/Providers/DbDataReaderAccessor.cs:line 29
   at Xtensive.Orm.Rse.RecordSetReader.Prepare(Boolean executeAsync) in /_/Orm/Xtensive.Orm/Orm/Rse/RecordSetReader.cs:line 159
   at Xtensive.Orm.Rse.RecordSetReader.Create(EnumerationContext context, ExecutableProvider provider) in /_/Orm/Xtensive.Orm/Orm/Rse/RecordSetReader.cs:line 228
   at Xtensive.Orm.Linq.TranslatedQuery.ExecuteSequence[T](Session session, ParameterContext parameterContext) in /_/Orm/Xtensive.Orm/Orm/Linq/TranslatedQuery.cs:line 69
   at Xtensive.Orm.Linq.QueryProvider.<ExecuteSequence>g__ExecuteSequenceQuery|8_0[T](TranslatedQuery query, Session session, ParameterContext parameterContext) in /_/Orm/Xtensive.Orm/Orm/Linq/QueryProvider.cs:line 89
   at Xtensive.Orm.Linq.QueryProvider.Execute[TResult](Expression expression, Func`4 runQuery) in /_/Orm/Xtensive.Orm/Orm/Linq/QueryProvider.cs:line 107
   at Xtensive.Orm.Linq.QueryProvider.ExecuteSequence[T](Expression expression) in /_/Orm/Xtensive.Orm/Orm/Linq/QueryProvider.cs:line 92
   at Xtensive.Orm.Linq.Queryable`1.GetEnumerator() in /_/Orm/Xtensive.Orm/Orm/Linq/Queryable.cs:line 39

SELECT TOP 10
    [a].[c01umn]
FROM
(
    SELECT [b].[c01umn] AS [c01umn]
    FROM
    (
        SELECT [c].[Id],
               1 AS [TypeId],
               (CASE
                    WHEN (cast(0 as bit) = 1) THEN
                        -1 // failed without cast
                    ELSE
                        [c].[Structure_EnumField]
                END
               ) AS [c01umn]
        FROM [dbo].[Entity] [c]
    ) [b]
    GROUP BY [b].[c01umn]
) [a];

long enum is OK (LongEnumType : long)

  Query.All<Entity>()
    .GroupBy(it => it.Link == null 
             ? it.EnumField.Value 
             : (LongEnumType)(-1))
    .Select(it => it.Key)
    .Take(10)
    .ToArray();

SELECT TOP 10
    [a].[c01umn]
FROM
(
    SELECT [b].[c01umn] AS [c01umn]
    FROM
    (
        SELECT [c].[Id],
               1 AS [TypeId],
               (CASE
                    WHEN ([c].[Link] IS NULL) THEN
                        [c].[EnumField]
                    ELSE
                        CAST(-1 as BIGINT) // OK with cast
                END
               ) AS [c01umn]
        FROM [dbo].[Entity] [c]
    ) [b]
    GROUP BY [b].[c01umn]
) [a];

[ondrejtucny]

As far as I know .NET does the same thing, it casts short to int. What is 0 in the example above? Is it constant? What type it was on on the level of IQueryable query?

The value 0 was provided as a compile time constant: c.IdArch = 0.

By the way, cast to integer may be connected with the fact that literal parameters are by default integer, for example here … AND ( CAST([a].[id_arch] AS integer) = 0)) 0 is treated as integer value, I can only say for now is that code is generalized and probably affects values in different parts of statements. It is not important for comparisons to have left and right parts of the same type and a smallint value can be compared with an integer value but from expressions standpoint it is better be the same type.

It would be better to analyze the expression tree and do own type decisions. C#'s and SQL's type compatibility rules don't match, SQL being IMO more permissive. Hence, for example, in case of comparison operators the types of operands are decided bottom-up, and only at the comparison level a decision is made whether injecting type casts is necessary or not. Similarly, in case of the binary operators (e.g. +, -), first the type of operands is discovered bottom-up, then a decision of the resulting type made at the operator level (e.g. short + int → int), and then a top-down type compatibility check is made, again inserting type casts if and only if necessary. This way I think you can get rid of most unwanted type casts.