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

Remove calls to prefersSequences() #8870

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

derrabus
Copy link
Member

AbstractPlatform::prefersSequences() is deprecated and does not exist in DBAL 3. Let's not call it if it's not there.

@derrabus derrabus mentioned this pull request Jul 30, 2021
@derrabus derrabus force-pushed the improvement/prefers-sequences branch from bbb909c to ca8b67b Compare August 4, 2021 20:23
@beberlei beberlei added this to the 2.10.0 milestone Aug 5, 2021
@derrabus derrabus force-pushed the improvement/prefers-sequences branch from ca8b67b to 0abd654 Compare August 5, 2021 16:25
@derrabus derrabus force-pushed the improvement/prefers-sequences branch 2 times, most recently from 4d06b88 to 1765c4c Compare August 7, 2021 21:34
Signed-off-by: Alexander M. Turek <me@derrabus.de>
@derrabus derrabus force-pushed the improvement/prefers-sequences branch from 1765c4c to edaa05a Compare August 7, 2021 21:48
@derrabus
Copy link
Member Author

derrabus commented Aug 7, 2021

I've pushed a new attempt, this time with special handling for Postgres and Oracle. Those were the only DBAL platforms that preferred sequences.

Theoretically, this would still be a breaking change, if someone used a custom DBAL driver implementation (is this a thing?) that supports sequences and identity columns and prefers sequences.

@greg0ire
Copy link
Member

greg0ire commented Aug 7, 2021

Theoretically, this would still be a breaking change, if someone used a custom DBAL driver implementation (is this a thing?) that supports sequences and identity columns and prefers sequences.

Thanks for mentioning that, it sounds pretty unlikely.

@morozov
Copy link
Member

morozov commented Aug 9, 2021

Since there's an ongoing discussion in #8893, let's agree on something there first to make sure the changes here correspond to the agreement or explicitly agree that the discussion shouldn't hold the merge. Except for #8870 (comment), looks good to me.

@greg0ire
Copy link
Member

greg0ire commented Aug 9, 2021

I'd say #8893 shouldn't hold the merge, these problems look independent to me.

@greg0ire greg0ire merged commit 7c15937 into doctrine:2.10.x Aug 10, 2021
@greg0ire
Copy link
Member

Thanks @derrabus !

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.

4 participants