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

Fix incorrect handling of transactions using deferred constraints #7556

Open
wants to merge 1 commit into
base: 2.10.x
Choose a base branch
from
Open

Fix incorrect handling of transactions using deferred constraints #7556

wants to merge 1 commit into from

Conversation

grongor
Copy link

@grongor grongor commented Jan 4, 2019

Q A
Type bug
BC Break no
Fixed issues #7555

Summary

Rollback called when outside of a transaction when using deferred constraints (PostgreSQL).

(I wasn't sure what branch to target, please tell me if I should change it :) Thanks )

@grongor
Copy link
Author

grongor commented Jan 4, 2019

Build will fail until we merge the DBAL changes. You can see successful build here https://travis-ci.org/grongor/doctrine2/builds/475297482 (it uses patched DBAL from here doctrine/dbal#3424 )

@Ocramius
Copy link
Member

Ocramius commented Jan 4, 2019

This looks like a much better approach than the one we discussed before: nice!

@Ocramius Ocramius added the Bug label Jan 4, 2019
@Ocramius Ocramius added this to the 2.7.0 milestone Jan 4, 2019
@Ocramius Ocramius self-assigned this Jan 4, 2019
@Ocramius
Copy link
Member

Ocramius commented Jan 4, 2019

Assuming doctrine/dbal#3424 can target 2.9.3, we need to bump the dependency constraint here. Since we can only bump it in a minor release, and 2.7 is closed for improvements (besides deprecations), I'm not sure if this should target 3.0.

/cc @doctrine/doctrinecore?

@grongor
Copy link
Author

grongor commented Jan 4, 2019

Shouldn't this be merged into older versions too, as this fixes a bug and is non-BC? Just saying ... I'm not familiar with your release schedules :)

@Ocramius
Copy link
Member

Ocramius commented Jan 4, 2019

Can't merge in older versions due to the dependency bump, and dependency bumps are off-limits for patch releases, unless it's a security issue.

@lcobucci
Copy link
Member

This is quite good for 2.7.0 just needs a rebase and the dependency bump on DBAL.

@ostrolucky ostrolucky assigned ostrolucky and unassigned ostrolucky Nov 15, 2019
@lcobucci
Copy link
Member

lcobucci commented Nov 15, 2019

@grongor @Ocramius I was wrong in my statement, DBAL still needs to be adjusted.

After talking to @morozov we've decided to pursue a simpler solution than the one proposed in doctrine/dbal#3424 (basically a try/finally in the Connection#commit()). I'll create a PR for that adding the necessary cross-platform tests.

However, DBAL 2.9 isn't maintained any longer and 2.10 requires PHP 7.2. Since we want to have ORM 2.7.0 out ASAP due to SF5 support I'll push this to ORM 2.8.0.

I hope you understand my decision ❤️

@lcobucci lcobucci modified the milestones: 2.7.0, 2.8.0 Nov 15, 2019
@beberlei beberlei modified the milestones: 2.8.0, 2.8.1, 2.8.2 Dec 4, 2020
@beberlei beberlei removed this from the 2.8.2 milestone Dec 13, 2020
@simPod
Copy link
Contributor

simPod commented Aug 16, 2021

@lcobucci is this superseded by doctrine/dbal#3424?

@lcobucci
Copy link
Member

@simPod it's been a while but that's correct. The challenge is to find a cross-platform implementation (that's well tested).

@greg0ire greg0ire force-pushed the 2.7 branch 3 times, most recently from e531738 to 66c95a6 Compare December 1, 2021 20:54
@greg0ire greg0ire changed the base branch from 2.7 to 2.10.x December 28, 2021 22:49
Copy link
Contributor

There hasn't been any activity on this pull request in the past 90 days, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
If you want to continue working on it, please leave a comment.

@github-actions github-actions bot added the Stale label Dec 31, 2024
@simPod
Copy link
Contributor

simPod commented Jan 5, 2025

Fixed in doctrine/dbal#6545

@github-actions github-actions bot removed the Stale label Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants