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

[4.1.0] experience report updating Marten #2607

Closed
baronfel opened this issue Sep 5, 2019 · 9 comments · Fixed by #2625

Comments

@baronfel
Copy link

commented Sep 5, 2019

Steps to reproduce

I was initially tinkering with updating Npgsql only to 4.1.0 in my application and ran into the first issue below, so I then tried to update Marten to use the 4.1.0-preview2 and noticed another change that I wanted to bring to your attention that prevent drop-in updates, in case some were unintentional:

The issue

Again, these may be entirely intended changes, I just wanted to bring them up to get an explicit yes or no on that point.

Further technical details

Npgsql version: 4.1.0-preview2
PostgreSQL version: 10
Operating system: Mac OSX Mojave 10.14.6

@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

Thanks for the report! This changes are not intentional and we should keep binary compatibility until version 5.

@baronfel

This comment has been minimized.

Copy link
Author

commented Sep 5, 2019

They seem like rather simple fixes, if you like I could prepare a PR with just these two changes.

@baronfel

This comment has been minimized.

Copy link
Author

commented Sep 6, 2019

I found another while testing the Npgsql.EntityFrameworkCore 2.2.4 package:

  • the member Npgsql.TypeHandling.NpgsqlTypeHandlerFactory Npgsql.TypeMapping.NpgsqlTypeMapping.get_TypeHandlerFactory() doesn't exist anymore. Instead, the return type of the getter is Npgsql.TypeHandling.NpgsqlTypeHandlerFactory. The result of this is that using 4.1.x with Npgsql.EntityFramework 2.x fails due to MethodNotFound exceptions.

This one I'm not so sure about how to fix. The others were purely additive, this one implies some larger architecture change from concrete classes to interface return types. The abstract NpgsqlTypeHandlerFactory type is ust gone now.

@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Sep 7, 2019

Type handling is a different story. While it's a public API there is no guaranties that breaking changes here will happen only in major releases, but since it breaks EF 2.x we should investigate probably on EF side. Will try to take a look ASAP.

@YohDeadfall YohDeadfall added this to the 4.1 milestone Sep 7, 2019

@YohDeadfall YohDeadfall added the bug label Sep 7, 2019

@roji

This comment has been minimized.

Copy link
Member

commented Sep 8, 2019

Agree that Npgsql 4.1 should definitely work with EF 2.x - a separate issue for this may be better. @YohDeadfall let me know what you find, otherwise I'll take a look.

@roji

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

The NpgsqlTypeMapping.TypeHandlerFactory issue was introduced in 8691e26: the property's type was changed from NpgsqlTypeHandlerFactory to INpgsqlTypeHandlerFactory. I'll revert this change soon.

BTW it won't be possible to use EF Core 2.x with Npgsql.NetTopologySuite 4.1.0 - specifically the spatial plugin (see #1023). This is expected and there's no way around it. Will add a breaking change note in the release notes.

@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

@roji If you have time to take this, do it. I'll try to concentrate on other issues assigned to me and related to the ADO.NET driver.

@roji

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

Sure thing, I'll take care of it soon. We'll probably revert the entire change.

@roji roji self-assigned this Sep 18, 2019

roji added a commit that referenced this issue Sep 18, 2019
Revert breaking change in NpgsqlTypeMapping.TypeHandlerFactory
Its type is now NpgsqlTypeHandlerFactory again, instead of
INpgsqlTypeHandlerFactory.

Fixes #2607

@roji roji closed this in #2625 Sep 18, 2019

roji added a commit that referenced this issue Sep 18, 2019
Revert breaking change in NpgsqlTypeMapping.TypeHandlerFactory
Its type is now NpgsqlTypeHandlerFactory again, instead of
INpgsqlTypeHandlerFactory.

Fixes #2607
@baronfel

This comment has been minimized.

Copy link
Author

commented Sep 18, 2019

Thanks @roji and @YohDeadfall for your work here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.