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

Deprecate not passing parameter type to bindParam() and bindValue() #5558

Merged

Conversation

morozov
Copy link
Member

@morozov morozov commented Jul 31, 2022

Q A
Type deprecation

The default for the parameter type primarily exists to save keystrokes for end users and works because in most cases binding an integer as a sting is okay for the underlying database platform and/or driver. It is fine to have such defaults in the wrapper-level "shortcut" methods like executeQuery($sql, $params) but they shouldn't be spread across different APIs at multiple architecture levels.

The ability to omit parameter binding type at the driver level is a PDO legacy which is a user-facing API as such but is a driver in the context of the DBAL.

Similar to #5556, this should have a minimal impact on the ORM and immediate users of the DBAL.

@morozov morozov added this to the 3.4.0 milestone Jul 31, 2022
@morozov morozov force-pushed the deprecate-not-passing-bound-parameter-type branch from ebb805c to 960e605 Compare July 31, 2022 18:13
@morozov morozov marked this pull request as ready for review July 31, 2022 18:26
@morozov morozov force-pushed the deprecate-not-passing-bound-parameter-type branch from 960e605 to 846718a Compare July 31, 2022 21:52
@derrabus
Copy link
Member

derrabus commented Aug 1, 2022

It is fine to have such defaults in the wrapper-level "shortcut" methods like executeQuery($sql, $params)

Doesn't this also apply to the wrapper-level Statement::bind*() methods? Those are called from userland code and I'm afraid we would make those calls unnecessarily verbose.

I totally agree to remove the defaults from the driver API though.

@morozov
Copy link
Member Author

morozov commented Aug 1, 2022

Doesn't this also apply to the wrapper-level Statement::bind*() methods? Those are called from userland code and I'm afraid we would make those calls unnecessarily verbose.

I believe there's no good reason to require types at the wrapper level. I'll revert the changes in the wrapper.

@morozov morozov force-pushed the deprecate-not-passing-bound-parameter-type branch 2 times, most recently from fff34e0 to 8fe2d78 Compare August 1, 2022 17:09
@morozov morozov force-pushed the deprecate-not-passing-bound-parameter-type branch from 8fe2d78 to ab2ad39 Compare August 1, 2022 18:37
@morozov morozov merged commit bf648df into doctrine:3.4.x Aug 1, 2022
@morozov morozov deleted the deprecate-not-passing-bound-parameter-type branch August 1, 2022 22:19
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants