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

Upgrade to 3.0.0 #822

Merged
merged 79 commits into from
Sep 20, 2019
Merged

Conversation

lauxjpn
Copy link
Collaborator

@lauxjpn lauxjpn commented Sep 18, 2019

This PR updates the provider and most tests to EF Core 3.0.
It is based on #820 and @twsl's extensive work from #818 and was also discussed on #797.

It should be noted for major updates in the future, that directly merging the SQL Server files (or parts of it) in this repository, is not the optimal way to proceed here, because it leads to many subtle changes in SqlFunctionExpression parameter orders and small behavior differences, that are hard to track down.
Instead, the updates should be triggered by failing tests, checking the providers log, comparing changes between different versions of the Sqlite and NpSql providers and applying the necessary changes manually.

On the plus side, having done this now once, the file and code structure is now much closer to those of other providers and should be easier to maintain in the future. I suggest keeping it this way.

The following table shows the current test metrics:

Version Total Passed Failed Skipped
2.2.6 8886 8743 12 131
3.0.0 14102 13389 237 476

Some tests that passed in the previous releases fail now by design/default, due to missing native support in MySQL for an underlying feature, that would implicitly trigger client eval in previous versions (see #821 for further discussion).
These include all EXCEPT, INTERSECT and OUTER APPLY queries.
Some test queries reference columns from an outer table and fail.
It might be possible to implement the latter and the OUTER APPLY functionality (at least for some cases) with the new LATERAL keyword (MySQL 8.0.14). This needs further research.

NO_BACKSLASH_ESCAPES is still disabled.

I haven't checked yet, if there are additional test classes to implement for 3.0.

The dependency versions should be updated shortly to public and stable releases.

I kept the original commit structure intact, to make it easier to review. All or most commits should be squashed when merging.

Everybody should feel encoraged to test/review this PR, so we can get this out there for people to test their own projects against and provide additional feedback.

I will continue fixing tests and isolating those, that depend on not implemented features.

I updated the CI files to support 3.0.

lauxjpn and others added 30 commits September 13, 2019 18:51
…e previous default setting of 30 seconds. Use 600 seconds (SQL Server provider default) instead. This will also be applied to queries setting up the database test environment.
Fix many issues responsible for previously failed tests.
Reactivate previously skipped tests, that work fine now.
Disable VisitSqlParameter NO_BACKSLASH_ESCAPES for the moment and update later, if necessary.
Versions need to be refactored into build variables before PR.
Implement DateTime.Millisecond support.
@caleblloyd caleblloyd changed the base branch from 2.2.6-wip to 3.0.0-wip September 20, 2019 05:37
@caleblloyd
Copy link
Contributor

Thanks for all of your work I'd like to get this merged into 3.0.0-wip

I changed CI to use Azure DevOps in #823. Can you merge master into your branch and update CI?

I was hoping we could move to full test coverage on Azure DevOps, but if you want to make it test a subset as master does now we can work that in a later PR

Once this is in the WIP branch I can setup Azure Devops to deploy nightlies off the WIP branch so people can start testing it out

@lauxjpn
Copy link
Collaborator Author

lauxjpn commented Sep 20, 2019

I will be merging this in 6 to 10 hours.

@lsvMoretti
Copy link

Would this fix my issue I have here?

#825

@lauxjpn
Copy link
Collaborator Author

lauxjpn commented Sep 20, 2019

@lsvMoretti Probably. Using a 2.2.x provider with EF Core 3.0 will not work. Your error seems to be related to some dependency mismatches between EF Core 3.0 and the provider.
Tryout the new 3.0 branch once merged and let us know.

@lauxjpn
Copy link
Collaborator Author

lauxjpn commented Sep 20, 2019

@caleblloyd Do you want me to squash all commits during the merge, squash them manually into a reasonable chunks (~10) before merging, or leave them as is and merge without squashing at all?

@caleblloyd
Copy link
Contributor

Just hit the squash and merge button on the PR

@lauxjpn
Copy link
Collaborator Author

lauxjpn commented Sep 20, 2019

I updated the Azure DevOps yaml to use .NET Core 3.0 and skip all tests for now, to merge this quickly as a WIP.
After merging, I will open another issue for keeping track on the newly implemented features, their supporting test, missing features and the remaining failing tests.

At this point in time, the test metrics look like this:

MySQL Total Passed Failed Skipped
8.0.17 14096 13472 145 479
5.7.27 14096 13421 187 488

The difference comes mainly from PARTITION BY and the newly supported LATERAL support in MySQL 8 (though some tests fail due to MySQL bugs I reported).

Careful with migrations. They might not work fully yet.

@lauxjpn lauxjpn merged commit 3f7da2a into PomeloFoundation:3.0.0-wip Sep 20, 2019
@lauxjpn lauxjpn deleted the upgrade_to_3.0.0 branch September 20, 2019 23:07
@lauxjpn lauxjpn self-assigned this Nov 1, 2019
@lauxjpn lauxjpn added this to the 3.0.0 milestone Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants