-
Notifications
You must be signed in to change notification settings - Fork 932
Improve support of Npgsql 4 #1905
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
Conversation
We have some compatibility issues with the newer driver, so need to upgrade
|
Most likely changes required are similar to nhibernate/nhibernate-envers#2 |
|
So, Npgsql 4 still has issues with connections released from the second phase of a system transaction commit/rollback.
As suggested here, this PR may have to introduce a new (TestDialect?) property for ignoring tests involving connections used in commit phases. Then something like the following could be added into protected override bool AppliesTo(Dialect.Dialect dialect)
=> (TestDialect.SupportsConnectionUseOnSystemTransactionPrepare ||
!UseConnectionOnSystemTransactionPrepare) &&
base.AppliesTo(dialect); |
fredericDelaporte
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a bit mixed feelings about this change.
On the one hand, we already do this on Get, so doing this on Set is more consistent.
On the other hand, adding our own conversion here while all the other data providers do no require it and handles it themselves is likely to be an overhead for those other data-providers. They will still check the value type, and in some case even re-convert it. (That will be the case for Oracle with current NHibernate Double registration, which actually maps it to a numeric (or double precision which translates to numeric) instead of binary_double available since 10g.)
Since Npgsql 3.2.5, system transactions support has regressed. Those tests were succeeding with Npgsql 3.2.4.1
Required TestDialect properties and ignore logic were already in master.
|
I think for the most of the providers this will be no-op as types do typically match to the expected type. Also, most of the types already do some conversation on set, so it is not a big deal. |
We have some compatibility issues with the newer driver, so need to upgrade