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.

Sign up for GitHub

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

Jump to bottom

Custom PDO fetch modes and 2.7.0 #3082

Closed
corphi opened this issue Apr 4, 2018 · 8 comments · Fixed by #3088
Closed

Custom PDO fetch modes and 2.7.0 #3082

corphi opened this issue Apr 4, 2018 · 8 comments · Fixed by #3088
Assignees
@Majkl578
Labels
BC Break Bug Missing Tests PDO Regression
Milestone
2.7.1

Comments

@corphi
Copy link
Contributor

corphi commented Apr 4, 2018

Announced as a deprecation only, an exception is thrown when passing unrecognized fetch modes, such as PDO::FETCH_KEY_PAIR. That is a BC break. In another instance, a silenced deprecation warning is issued instead.

While I completely agree on deprecating implicit usage of PDO features, I'd like a migration path that does not involve breaking working code. I suppose that some code depends on recognized fetch modes; I will investigate this and want to provide a pull request.
@Ocramius
Copy link
Member

Ocramius commented Apr 4, 2018

@corphi unlikely happened intentionally: if you have a piece of code that reproduces the regression, we can make a patch release with a fix.

@Ocramius Ocramius added this to the 2.7.1 milestone Apr 4, 2018
@alongosz
Copy link

alongosz commented Apr 4, 2018

@Ocramius for us it also breaks a piece of code.
We have the usage:
$stmt->fetchAll(PDO::FETCH_ASSOC | PDO::FETCH_GROUP) (it was fine since PDOStatement::fetchAll doc allowed bitwise-ORs for the fetch_style argument).

While we can of course refactor the code to use FETCH_ASSOC only and some array_* magic, I'm rather still in favor of silenced deprecation warning instead :)

@Ocramius
Copy link
Member

Ocramius commented Apr 4, 2018

Uhhh, I think we never considered bitmasking fetch modes on our side, so that is most likely lack of knowledge on our end.

@danrot danrot mentioned this issue Apr 4, 2018
Merged
2 tasks
@dbu
Copy link
Member

dbu commented Apr 4, 2018

We see the problem coming up in PHPCR-ODM (because dbal is used in jackalope-doctrine-dbal) as well: https://travis-ci.org/doctrine/phpcr-odm/jobs/361545802#L474

Is there an easy fix for this? And should we change code that relied on this, or is it an intended feature?

@andrerom
Copy link

andrerom commented Apr 4, 2018

Same here, seeing the bitmasking fetch modes hitting us, breaking stable releases out there.

@Ocramius
Copy link
Member

Ocramius commented Apr 4, 2018

Test cases please: adding "me too" is NOT useful

@Majkl578
Copy link
Contributor

Majkl578 commented Apr 4, 2018

Likely side-effect of #2996.

@corphi corphi mentioned this issue Apr 5, 2018
Merged
Majkl578 added a commit that referenced this issue Apr 7, 2018
@Majkl578
Merge pull request #3088 from corphi/fix-pdo-fetch-mode-bc
d4a96de 
Fix #3082: Add BC for PDO-only fetch modes
@Majkl578 Majkl578 self-assigned this Apr 7, 2018
rgrellmann added a commit to Rossmann-IT/dbal that referenced this issue Apr 13, 2018
@rgrellmann
Merge tag 'v2.7.1' into oracle121
8e67496 
This release fixes unintentional BC breaks:

1. It was impossible to use deprecated fetch modes with PDO-based drivers.
2. An unsupported option passed to the `Column` object prevented subsequent options from being applied.
3. Date interval values stored prior to upgrade to `v2.7.0` were reported as invalid.

Total issues resolved: **10**

**Backwards Compatibility Fixes:**

- [3082: Custom PDO fetch modes and 2.7.0](doctrine#3082) thanks to @corphi
- [3088: Fix doctrine#3082: Add BC for PDO-only fetch modes](doctrine#3088) thanks to @corphi
- [3089: Don't skip column options.](doctrine#3089) thanks to @andytruong
- [3093: When updating to version v2.7 type DateInterval throws errors](doctrine#3093) thanks to @fnagel
- [3097: Fix compatibility for pre-2.7 DateIntervalType format](doctrine#3097) thanks to @Majkl578

**Documentation Improvements:**

- [3083: Document the correct way of configuring a MariaDB database with serverVersion](doctrine#3083) thanks to @tristanbes
- [3084: README: Add 2.7, drop 2.5](doctrine#3084) thanks to @Majkl578

**Continuous Integration Improvements:**

- [3085: Tests: remove implicit verbose flag](doctrine#3085) thanks to @Majkl578
- [3090: Add symfony tests listener](doctrine#3090) thanks to @greg0ire
- [3095: CI: Add missing listener for MariaDB @ mysqli](doctrine#3095) thanks to @Majkl578

# gpg: directory `/n/.gnupg' created
# gpg: new configuration file `/n/.gnupg/gpg.conf' created
# gpg: WARNING: options in `/n/.gnupg/gpg.conf' are not yet active during this run
# gpg: DBG: locking for `/n/.gnupg/pubring.gpg.lock' done via O_EXCL
# gpg: keyring `/n/.gnupg/pubring.gpg' created
# gpg: Signature made Sun Apr  8 07:24:49 2018     using RSA key ID 543AE995
# gpg: Can't check signature: public key not found

# Conflicts:
#	.gitignore
#	lib/Doctrine/DBAL/Driver/OCI8/Driver.php
@chrisguitarguy chrisguitarguy mentioned this issue Apr 18, 2018
Closed
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Majkl578 Majkl578

Labels
BC Break Bug Missing Tests PDO Regression
Projects
None yet
Milestone
2.7.1
Development

Successfully merging a pull request may close this issue.

Fix #3082: Add BC for PDO-only fetch modes corphi/dbal
6 participants
@dbu @corphi @Majkl578 @Ocramius @andrerom @alongosz