CirrusRedOrg / EntityFrameworkCore.Jet

Entity Framework Core provider for Access database
MIT License
138 stars 38 forks source link

EF Core 7 support #132

Closed ChrisJollyAU closed 2 months ago

ChrisJollyAU commented 1 year ago

With .Net 7 and Entity Framework Core 7 out now I have created this topic for any questions regarding the status of EFCore.Jet for EF 7

ChrisJollyAU commented 1 year ago

@nicovil Use this topic for any EF 7 questions

nicovil commented 1 year ago

@ChrisJollyAU I just cloned your branch (I didn't knew you had one for .net 7).

I noticed that the dependencies.targets file is outdated, so I changed DotnetRuntimeVersion dependency from 6.0.10 to 7.0.0, updated package references to their latest versions that belongs to net 7.0 (except Microsoft.Win32.Registry that stills being in preview build for 6.0), and ran nuget update.

I made a build and everything compiled flawlessly, so great work!

I will be doing more tests soon.

Of course there are unit tests that are not passing, but they're near 10000 unit tests failing, it's a nightmare to fix them all!

ChrisJollyAU commented 1 year ago

@nicovil I had just created the branch this week. Not sure if I use that variable anywhere. I have a tendency to update the package from the VS package manager rather than use those variables.

As to the test, I have around 50% pass rate. Some obviously can be fixed as they are just EF 7 optimizing the structure of the query in a slightly different way

P.S. Just updated to the GA release from using the RC2

ShamilS commented 1 year ago

@nicovil Use this topic for any EF 7 questions

@ChrisJollyAU : How to migrate bool data type to Yes/No field?

I'm using EntityFrameworkCore.Jet (EF7) and I have a model class with a bool data type:

public class MyTestClass
{
    ...
    public bool BoolField {get;set;}
    ... 
}

When creating and applying a migration involving this class the target BoolField field in the target MS Access MyTestclass table is created as having Number (Integer) data type. How to make it created as a Yes/No (Bool) MS Access data type ?

ChrisJollyAU commented 1 year ago

@ShamilS Have had a look. Looks like the CLR type bool is currently mapped to a smallint type on the database side. Which would be why it is currently showing like it is.

It would be possible to change that to map to a bit instead (bit,boolean,logical are all synonyms for ms access). However this might introduce a new pronblem. In the file that sets up all this mapping JetTypeMappingSource.cs there is a comment next to the mapping for the bit type that JET bits are not nullable. So it is either True or False. Only 2 options. This has the potential to break anything if you are using a nullable bool (bool?)

If using it as a number type, you can do 0,1 or NULL

May need some further checking on this

ShamilS commented 1 year ago

@ShamilS Have had a look. Looks like the CLR type bool is currently mapped to a smallint type on the database side. Which would be why it is currently showing like it is. ... In the file that sets up all this mapping JetTypeMappingSource.cs there is a comment next to the mapping for the bit type that JET bits are not nullable

@ChrisJollyAU Thank you for your clarification. I have had a look at JetTypeMappingSource.cs . I see what you mean.

Is it possible to modify the current EFCore.Jet (EF7) codebase to optionally force migration procedure to set the target database field type to "Bool (Yes/No)" by using DataTypeAttibute, e.g.,

....
[DataType("Yes/No")]
public bool BoolField {get; set; }
....

?

ChrisJollyAU commented 1 year ago

Maybe. Off hand I don't think it will be too simple.

Is there a specific reason you need it to be a proper MS Access Yes/No Boolean?

PedroGGaspar commented 1 year ago

@ChrisJollyAU, I don't know the reason for @ShamilS wanting a bit field, but I would guess it's about storage space. The smallint needs 2 bytes while a bit field needs just 1 byte. Couldn't the bool type map to byte/tinyint then?

ShamilS commented 1 year ago

Sorry, I didn't reply promptly, I was busy with a project deadlines.

Is there a specific reason you need it to be a proper MS Access Yes/No Boolean?

@ChrisJollyAU No, I can live with it.

ShamilS commented 1 year ago

@ChrisJollyAU, I don't know the reason for @ShamilS wanting a bit field, but I would guess it's about storage space. The smallint needs 2 bytes while a bit field needs just 1 byte. Couldn't the bool type map to byte/tinyint then?

@PedroGGaspar No, it wasn't about storage space preservation. I missed the point, which @ChrisJollyAU pointed me to, that it would be useful to have ternary logic handled by nullable bool?-type fields and this data with 'null' values Boolean fields should be possible to be saved to/retrieved from MS Access backend handled by the EntityFrameworkCore.Jet7.

P.S. Although, if if that wouldn't break/affects heavily EntityFrameworkCore.Jet versions compatibility I'd not mind to have C#'s bool?-data type to be mapped to MS Access's Number/Byte.

ShamilS commented 1 year ago

I'm getting a runtime error while trying to save simple data entity instance:

Unhandled exception. Microsoft.EntityFrameworkCore.DbUpdateException: 
 An error occurred while saving the entity changes. See the inner exception for details.
  ---> System.Data.OleDb.OleDbException (0x80040E10): Too few parameters. Expected 8.

I'm getting this error for an entity type with nullable fields:

public class TestClass3
{
    public int Id { get; set; }
    public bool? BoolField {get; set; }
    public char? CharField { get; set; }
    public decimal? DecimalField { get; set; }
    public double? DoubleField { get; set; }
    public float? FloatField { get; set; }
    public short? ShortField { get; set; }
    public int? IntField { get; set; }
    public long? LongField { get; set; }
}

and all the data fields' values are set.

This error happens in EFCore.Jet.Data.JetCommand.cs

    protected virtual int ExecuteNonQueryCore()
    {
         ...            
         return _connection.RowCount = InnerCommand.ExecuteNonQuery();
    }

and the ExecureNonQuery() is located in the same JetCommand.cs class but somehow can't be debugged/traced:

    public override int ExecuteNonQuery()
    {
        if (Connection == null)
            throw new InvalidOperationException(Messages.PropertyNotInitialized(nameof(Connection)));

        ExpandParameters();

        return SplitCommands()
            .Aggregate(0, (_, command) => command.ExecuteNonQueryCore());
    }

The SQL command built by EFCore.Jet is:

"INSERT INTO `TestClasses2` (`BoolField`, `CharField`, `DecimalField`, `DoubleField`, 
         `FloatField`, `IntField`, `LongField`, `ShortField`)\r\nVALUES (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7)"

and all parameters and their values seems to be in place - stored in InnerCommand.Parameters.

I'm defining MS Access backend connection as

 var dbFullPath = @"G:\Projects\OSS\NW_NET_Access\Database\TestClassDb.mdb";
 var dbOptions = new DbContextOptionsBuilder<MyDataContext>()
    .UseJet($@"Provider=Microsoft.ACE.OLEDB.12.0;Data Source={dbFullPath};")
    .Options;
 using (var ctx = new MyDataContext(dbOptions))
 {
   ...

at the top level of my sample code - and it works well for the sample data retrieval but fails for the sample data insert.

When a sample class has all not-nullable data fields:

public class TestClass4
{
    public int Id { get; set; }
    public bool BoolField {get; set; }
    public char CharField { get; set; }
    public decimal DecimalField { get; set; }
    public double DoubleField { get; set; }
    public float FloatField { get; set; }
    public short ShortField { get; set; }
    public int IntField { get; set; }
    public long LongField { get; set; }
}

then runtime error is:

 Data type mismatch in criteria expression.
ChrisJollyAU commented 1 year ago

@ShamilS It's an issue with decimal types. In fact it's been around for a while. See issue #32

ShamilS commented 1 year ago

@ShamilS It's an issue with decimal types. In fact it's been around for a while. See issue #32

@ChrisJollyAU OK, I guess you mean the "Data Type Mismatch in criteria expression" for the case when all entity's fields are not-nullable? Actual SQL is

    "INSERT INTO `TestClasses2` (`BoolField`, `CharField`, `DecimalField`, `DoubleField`, 
         `FloatField`, `IntField`, `LongField`, `ShortField`)\r\nVALUES (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7)"

so the issue isn't that much OleDb-related, which uses English-US numbers (and dates) formatting styles/separators, but is somewhere in the EFCore.Jet code, where parameters' values are set. So this issue can be fixed I suppose when its time to be fixed comes.

As for "Too few parameters. Expected {x}" runtime error when an entity's fields are all nullable but values are all set before saving - has this issue also been mentioned already?

ChrisJollyAU commented 1 year ago

Quick progress update

  1. Enabled nullable on the EFCore.Jet src projects. Tests are not enabled (same as in sqlserver tests)
  2. Fixed the Math and String translators. Updated to how current Sql Server generates things. This fixes some tests (both by generating a sql server style code that works as well as handling scenarios not previously handled)
  3. Fixed a number of NorthWind* tests (Mostly in NorthwindFunctions, NorthwindGroupBy, NorthwindAggregate)
ChrisJollyAU commented 1 year ago

Hi All

Another progress update

  1. Have merged the EF Core 7 code into the master branch. For the first time in a long time the current branch is targeting the current version of EF Core
  2. Fixed more tests
  3. Fixed the Dual table queries
  4. Some other smaller fixes

v7 Alpha 1 should appear in the daily build feed. @bubibubi or @lauxjpn Can you check out the nuget feed. It looks like the API key has expired so the script is failing to push to nuget

On a different note, I do have a question regarding DateTimeOffset support.

EFCore.Jet maps it to a DateTime column in the database. MS Access has a lower value of 1 Jan 100 but the EFCore tests have a lot that use DateTime and DateTimeOffset with values less than that. Currently these tests are failing but fixing it could fix around 1200 - 1500 tests. Any ideas on how we should handle this?

lauxjpn commented 1 year ago

Can you check out the nuget feed. It looks like the API key has expired so the script is failing to push to nuget

@ChrisJollyAU I updated the nuget.org API key and reran the publishing job. The 7.0.0-alpha.1 release is now published on nuget.org, as is 6.0.0-alpha.2.

I also sent you an invite to your GitHub email address for our AZDO team.

On a different note, I do have a question regarding DateTimeOffset support.

Just post me the names of two of those tests in question and I'll take a look at it.

ChrisJollyAU commented 1 year ago

@lauxjpn Any test from the Gears of War test set (GearsOfWarQueryJetTest) will work. Same problem for all 1265 tests as problem comes in during the initial seeding

I also sent you an invite to your GitHub email address for our AZDO team.

Haven't seen any email yet

ChrisJollyAU commented 1 year ago

@lauxjpn Just a bit more info. The DateTime problem seems to be a Jet ODBC driver issue. I was testing with other dates and still had the same problem even though the date was within MS Access limits. But some dates still did work.

It turns out Jet ODBC doesn't like dates before 1 Jan 1753! I don't believe there is a workaround. See also this archived KB article from 2001 https://jeffpar.github.io/kbarchive/kb/252/Q252699/

This appears to be a Jet ODBC specific issue, as switching to OleDb had more tests passing that used DateTime than with Odbc

On that note the decimal type also works with OleDb but not Odbc

Can I ask if there is a specific reason why the default provider is Odbc? Probably would be better to switch to OleDb as the default

ChrisJollyAU commented 1 year ago

Alpha 2 is now out

Notable improvements are:

ChrisJollyAU commented 1 year ago

Beta 1 is now out

Improvements:

ChrisJollyAU commented 1 year ago

RC 1 is now out

Improvements:

Think we are getting close to a stable release. Most fixes are starting to take a lot longer to sort out. Unless there is anything major, this should be almost ready for rtm

@bubibubi @lauxjpn (And anyone else that wants to) Can you take a look at the tests and see if there is anything else of importance to fix before rtm. Most failing tests now seem to be more due to Jet limitations (concurrency, datetimeoffsets, no date before 100 years, APPLY joins etc)

ChrisJollyAU commented 1 year ago

v7 RTM is out!!

Improvements:

angelofb commented 1 year ago

something wrong

❯ dotnet add package EntityFrameworkCore.Jet.Odbc --version 7.0.0 Determining projects to restore... ... error: NU1102: Unable to find package EntityFrameworkCore.Jet.Data with version (>= 7.0.0) error: - Found 12 version(s) in nuget.org [ Nearest version: 7.0.0-rc.1 ] error: - Found 0 version(s) in C:\Program Files\dotnet\library-packs ... error: Package 'EntityFrameworkCore.Jet.Odbc' is incompatible with 'all' frameworks in project

My project is net7.0

ChrisJollyAU commented 1 year ago

@angelofb Oh dear. It looks like nuget didn't get that one package. All the others got updated

image

Will take a look and try to republish

0xced commented 1 year ago

I just opened #139 which will solve the publishing issue.

But publishing only EntityFrameworkCore.Jet.Data 7.0.0 now that the other 3 NuGet packages are successfully published might be tricky with the current Azure pipeline definition. Maybe the easiest way is to publish version 7.0.1 ?

ChrisJollyAU commented 1 year ago

There we go. That should do the trick now. @angelofb should be fine now

@0xced Added some code to the pipeline yaml to detect a pack error and exit with an error rather than just ignore any errors and continue

angelofb commented 1 year ago

that was quick! it is ok now, thank you!

ChrisJollyAU commented 1 year ago

v7.0.2

Needed another update due to #140

The problem was that when referencing both Sql Server and Jet, there were some Fluent API extensions (for the KeyBuilder/ModelBuilder/PropertyBuilder objects) that shared the same definition (same name and parameters).

To be able to support referencing Jet with other providers the extension methods have needed a name change. At this point in time it looks like it's only UseIdentityColumn(s) -> UseJetIdentityColumn(s)

As a note looking at the EF Core repository, SQL Server, Cosmos and Sqlite have no extensions defined that have the same name between them