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 colon prefix for prepared statement parameters #4407

Merged

Conversation

morozov
Copy link
Member

@morozov morozov commented Nov 3, 2020

Q A
Type deprecation

Summary

The syntax with the leading colon exists for compatibility with PDO which is no longer the goal. Essentially, the colon is part of the SQL syntax for parameter placeholders, not part of the parameter name. It has certain downsides:

  1. It's ambiguous as it allows to bind both 'param' and ':param' to the same statement w/o the explicitly specified precedence.
  2. It requires extra effort when fixing certain issues (e.g. Incorrect handling of list parameters with sparse types #2897 is not fixed for named types because of the leading colon).

Apart from that, as part of fixing #4383 (or soon after that), I'd like to use the same SQL parser for the wrapper layer and the OCI8 statement. At that time, I'd like to not have to deal with supporting both syntaxes.

@SenseException
Copy link
Member

What about the usage of trigger_error for the color-prefix? A silent treatment might hit developers unprepared after updating to 3.0.

@beberlei
Copy link
Member

beberlei commented Nov 3, 2020

@SenseException we'll handle triggers in a seperate step later in 2.x cycle

UPGRADE.md Outdated Show resolved Hide resolved
@morozov morozov force-pushed the deprecate-named-param-colon-prefix branch from 02216c5 to 7b4f16a Compare November 3, 2020 20:51
@SenseException
Copy link
Member

@SenseException we'll handle triggers in a seperate step later in 2.x cycle

What is the reason/benefit for this separation?

@morozov
Copy link
Member Author

morozov commented Nov 4, 2020

What is the reason/benefit for this separation?

Reporting each bound parameter via trigger_error() may be too noisy and impact performance. The API Benjamin is working on won't have this downside.

@morozov morozov merged commit ff83e0b into doctrine:2.12.x Nov 4, 2020
@morozov morozov added this to the 2.12.1 milestone Nov 4, 2020
@morozov morozov deleted the deprecate-named-param-colon-prefix branch November 4, 2020 23:59
rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Mar 7, 2021
Release [2.12.1](https://github.com/doctrine/dbal/milestone/84)

2.12.1
======

- Total issues resolved: **2**
- Total pull requests resolved: **11**
- Total contributors: **7**

Documentation,Prepared Statements
---------------------------------

 - [4424: Mark SQLParserUtils internal](doctrine#4424) thanks to @morozov

Packaging
---------

 - [4416: Update .gitattributes](doctrine#4416) thanks to @bytestream

Bug,Cache
---------

 - [4414: ResultCacheStatement::fetchAllAssociative does not store results in cache](doctrine#4414) thanks to @morozov and @dFayet

Deprecation,Prepared Statements
-------------------------------

 - [4411: Deprecate inappropriate usage of prepared statement parameters](doctrine#4411) thanks to @morozov
 - [4407: Deprecate colon prefix for prepared statement parameters](doctrine#4407) thanks to @morozov

Static Analysis
---------------

 - [4403: Remove redundant phpstan param from DriverManager::getConnection()](doctrine#4403) thanks to @simPod

Bug,Locking,Transactions
------------------------

 - [4400: LockMode::NONE should not set WITH (NOLOCK)](doctrine#4400) thanks to @BenMorel

Code Style,PHP
--------------

 - [4398: Update PHP&doctrine#95;CodeSniffer to 3.5.8](doctrine#4398) thanks to @morozov

PDO,PHP,Test Suite
------------------

 - [4396: Fix php8 mysql mariadb](doctrine#4396) thanks to @greg0ire

Documentation
-------------

 - [4390: Fix headline in the upgrade docs](doctrine#4390) thanks to @jdreesen

Documentation,Testing
---------------------

 - [4356: Testing Guidelines](doctrine#4356) thanks to @morozov

# gpg: Signature made Sat Nov 14 21:50:01 2020
# gpg:                using DSA key 1BEDEE0A820BC30D858F9F0C2C3A645671828132
# gpg: Can't check signature: No public key

# Conflicts:
#	README.md
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2022
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.

4 participants