babelfish-for-postgresql / babelfish_extensions

Babelfish for PostgreSQL provides the capability for PostgreSQL to work with applications written for Microsoft SQL Server. Babelfish understands the SQL Server wire-protocol and T-SQL, the Microsoft SQL Server query and procedural language, so you don’t have to switch database drivers or rewrite all of your application queries.
https://babelfishpg.org/
Apache License 2.0
277 stars 93 forks source link

[Bug]: Implicit casting to TINYINT causes errors in Entity Framework #2987

Open staticlibs opened 1 month ago

staticlibs commented 1 month ago

What happened?

With the following table:

create table tab1 (
    id int,
    col1 tinyint
)
insert into tab1 values(42, 1)

The following C# application example with Entity Framework will throw an error on Babelfish intermittently depending on current time (and the presence of records in tab1 table):

using Microsoft.EntityFrameworkCore;

public enum MyEnum : byte
{
    None = 0,
    One = 1,
    Two = 2,
}

public class Tab1
{
    public int Id { get; set; }
    public MyEnum? Col1 { get; set; }
}

public class TestContext : DbContext
{
    const string connStr = "Server=192.168.178.58,1433;Initial Catalog=ef1;User ID=jdbc_user;Password=12345678;Trust Server Certificate=true";

    public DbSet<Tab1> Tab1 { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder options)
    {
        options.UseSqlServer(connStr);
    }
}

class Program
{
    static void Main()
    {
        using var db = new TestContext();
        bool externalFlag = DateTime.Now.Minute % 2 == 0;
        IQueryable<Tab1> query1 = from t in db.Tab1
        select new Tab1
        {
            Id = t.Id,
            Col1 = externalFlag ? (t.Id > 0 ? MyEnum.Two : null) : null
        };
        Console.WriteLine(query1.ToList().Count);
    }
}
> dotnet run
1
> dotnet run
Unhandled exception. System.InvalidCastException: Unable to cast object of type 'System.Int16' to type 'System.Byte'.
   at Microsoft.Data.SqlClient.SqlBuffer.get_Byte()
   at Microsoft.Data.SqlClient.SqlDataReader.GetByte(Int32 i)
   at lambda_method3(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Program.Main() in C:\projects\postgres\dotnet\ef3\Program.cs:line 44

In this example when externalFlag is set, EF will generate the following SQL:

SELECT [t].[Id], CASE
    WHEN [t].[Id] > 0 THEN CAST(2 AS tinyint)
    ELSE NULL
END AS [Col1]
FROM [Tab1] AS [t]

On MSSQL Col1 column is returned as tinyint, but on Babelfish it is returned as smallint. In EF/ADO.NET the tynyint DB type is mapped to byte C# type and smallint is mapped to short (reference). When records are read from a result-set there is no implicit conversion allowed between byte and short in either way.

The problem happens because tinyint in Babelfish is implemented as a DOMAIN so when implicit cast to tinyint happens - the base type smallint is used instead.

Besides CASE, the same problem can be reproduced with UNION:

create view view1 as
select id, col1 from tab1
union all
select 43 as id, null as col1
select data_type
from information_schema.columns
where table_name = 'view1'
and column_name = 'col1'
data_type
---------------
smallint

The problem happens not only with null as col1, but also with varchar conversion like '' as col1 or '42' as col1.

As a workaround it is possible to use the explicit cast to tinyint, it is trivial to do it in static T-SQL, and not so trivial in Entity Framework queries (can be done with user-defined function mapping - 1, 2).

I wonder if it is feasible to re-implement the tinyint in Babelfish as a proper TYPE instead of a DOMAIN without breaking compatibility with existing DBs that use tinyint columns?

Version

BABEL_4_X_DEV (Default)

Extension

babelfishpg_common

Which flavor of Linux are you using when you see the bug?

Fedora

Relevant log output

No response

Code of Conduct

Deepesh125 commented 1 month ago

We recently fixed similar bug around resultant data type / typmod of case expression. I will check with @Yvinayak07 whether that fix will resolve this issue.

staticlibs commented 1 month ago

I've checked that the change from #2931 does not fix this issue, but the new hook is called, so perhaps it can be modified to cover tinyint. If it is possible to also add a hook to cover UNION for tinyint - that would be great. There is currently no workaround to this problem without large scale changes to either client app (adding casts with user-defined function mapping to all queries) or to both app and DB (changing the column type to smallint in all tables).