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

EF Core 7 support #132

Open
ChrisJollyAU opened this issue Nov 9, 2022 · 28 comments
Open

EF Core 7 support #132

ChrisJollyAU opened this issue Nov 9, 2022 · 28 comments

Comments

@ChrisJollyAU
Copy link
Member

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
Copy link
Member Author

@nicovil Use this topic for any EF 7 questions

@nicovil
Copy link

nicovil commented Nov 9, 2022

@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
Copy link
Member Author

@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
Copy link

ShamilS commented Nov 16, 2022

@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
Copy link
Member Author

@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
Copy link

ShamilS commented Nov 17, 2022

@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
Copy link
Member Author

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
Copy link

@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
Copy link

ShamilS commented Nov 22, 2022

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
Copy link

ShamilS commented Nov 22, 2022

@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
Copy link

ShamilS commented Nov 22, 2022

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
Copy link
Member Author

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

@ShamilS
Copy link

ShamilS commented Nov 23, 2022

@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
Copy link
Member Author

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 ChrisJollyAU pinned this issue Dec 8, 2022
@ChrisJollyAU
Copy link
Member Author

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
Copy link
Member

lauxjpn commented Feb 20, 2023

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
Copy link
Member Author

@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
Copy link
Member Author

@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
Copy link
Member Author

Alpha 2 is now out

Notable improvements are:

  • Fixed replacing global variable placeholders
  • Added JetParameterBaseSqlProcessor and JetSqlNullabilityProcessor. Fixesthe order of how it optimizes nullable values
  • CreateDatabase now tries to use the current provider type instead of defaulting to Odbc
  • Add Floor and Ceiling to Math translations
  • A huge amount of tests were fixed

@ChrisJollyAU
Copy link
Member Author

Beta 1 is now out

Improvements:

  • Unrolled COALESCE with 3 or more arguments
  • Finally got the Gears of War tests working (needed a data change to get passed a rather strict foreign key set up)
  • Fixed the Object ToString translator to not handle some things on the server (like enums)
  • Fixed the Convert translator - was picking up a tostring method that required parameters whereas the server was only meant to handle the ones without parameters (often the parameters were a format string or format provider)
  • Fixed some null propagatability in LEN and MID functions (specially MID does not accept NULL for length parameter whereas SQL Server does)
  • Fixed reading TimeSpan from the database

@ChrisJollyAU
Copy link
Member Author

RC 1 is now out

Improvements:

  • Generate the correct SQL literal for GUID (with the curly braces)
  • Change the way ordering works with boolean types. Jet uses -1 for true and 0 for false. Previous behaviour changed whether that column was ordered ascending or descending. While that worked, if you had null values in amongst it the specific placements of those wasn't matching the expected. Keeping the original ASC or DESC but ordering by the NOT value of the boolean works better
  • Fixed escaping wild chars in a string literal when used with a pattern (using the LIKE clause so calls such as Contains, StartsWith,EndsWith)
  • Fix the GetValueGenerated function in the JetValueGeneratorConvention: Function arguments needed to be updated to the current EF Core. Now sets the ValueGenerated.OnAdd annotation for the identity
  • Remove sbyte from the clr Type Mappings. EF Core has inbuilt type conversions for types which includes sbyte,ushort,uint etc. These types can be mapped with the conversions (EF Core just upgrades to the next numeric type so sbyte is mapped to Int16). The set up for sbyte here was close but lacked the correct comparer/converter so at times when we were working with sbyte it was trying to compare with Int16. Using the inbuilt conversions EF Core sets that up automatically. This is the same behaviour in EFCore.SqlServer
  • CAST: Used to throw an error if there was a cast that couldn't be handled be the cast functions (CStr, CLng, CInt, CDate etc). Given that not everything is handled by that (e.g. no function to cast to GUID), we just ignore the cast and include the value/column directly. The way Jet/Ace operates it seems more flexible (especially when a number of the functions/operators take various column types including variants)
  • Indexes: Add a index convention to add a filter to filter out null values for unique indexes
  • Convention Set Builder: Updates to current EF Core style
  • Fixed some clr type issues when mapping store type of text. Make sure the clr type of the incoming type info is the same as the type mapping we return. Already had that check when dealing with System.String but not when dealing with store type text
  • Decimal types have a maximum precision of 20 and not 38 (Jet/Ace/MS Access limitation)
  • NEW OPTION: When configuring the Jet options there is a new option UseShortTextForSystemString. Current behaviour has been mapping System.String properties (given they have no size specificed) to an unbounded string and as such is mapped to a longtext/memo (SQL Server uses nvarchar(max) ). Jet as some problems with memo types in that you can't do a join on those columns and ordering can have problems. Set this option to map it to use the normal short text column type. Note that the short text has a maximum size of 255! This option is NOT enabled by default so nothing changes for those with current usage unless they specifically enable it

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
Copy link
Member Author

v7 RTM is out!!

Improvements:

  • Library now targets net6.0 instead of net6.0-windows . While it still uses a lot of Windows functionality (DAO,ADOX,OleDb) this change allows you to reference it in another multi-platform package and only expose the EFCore.Jet functionality if running on Windows. Thanks @0xced
  • Handle a left join that is immediately preceded by a cross join (otherwise known as cartesian product/join). For Jet the cross join needs to be pushed down into its own subquery before doing the left join
  • Changed some handling of the Jet conversion functions CDbl, CInt, Clng etc. These functions do not accept NULL values (unlike SQL Server CAST), thus the inside expression needs to be NULL checked. Included some exceptions if we can pick up that the inside function will never produce a NULL then we can simplify and do without the NULL check.
  • More test fixes

@angelofb
Copy link

angelofb commented Jul 3, 2023

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
Copy link
Member Author

@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
Copy link
Contributor

0xced commented Jul 3, 2023

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
Copy link
Member Author

ChrisJollyAU commented Jul 3, 2023

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
Copy link

angelofb commented Jul 3, 2023

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

@ChrisJollyAU
Copy link
Member Author

ChrisJollyAU commented Jul 3, 2023

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

@lauxjpn lauxjpn changed the title EF 7 support EF Core 7 support Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants