[RFC][VOTE] Change default PDO error mode

php.internals

AllenJB

6 years ago
Hi all, I have opened voting on the RFC to change the default PDO error mode: https://wiki.php.net/rfc/pdo_default_errmode As no concerns were raised during the discussion period, the RFC is unchanged from that originally posted. Previous discussion threads: * https://externals.io/message/109015 * https://externals.io/message/109398 Voting closes on April 27th. Regards, AllenJB

Christian Schneider

6 years ago
Am 13.04.2020 um 14:41 schrieb AllenJB <php.lists@allenjb.me.uk>:
> As no concerns were raised during the discussion period, the RFC is unchanged from that originally posted. > > Previous discussion threads: > * https://externals.io/message/109015
For the record: I did raise concerns in https://externals.io/message/109015#109144 and I still think that switching directly from silent mode to exceptions and thus changing the control flow without prior warning is not the way to go. That's why I voted no. Regards, - Chris

Derick Rethans

6 years ago
On Mon, 13 Apr 2020, Christian Schneider wrote:
> Am 13.04.2020 um 14:41 schrieb AllenJB <php.lists@allenjb.me.uk>: > > As no concerns were raised during the discussion period, the RFC is unchanged from that originally posted. > > > > Previous discussion threads: > > * https://externals.io/message/109015 > > For the record: I did raise concerns in > https://externals.io/message/109015#109144
You did, but you never followed up on his questions: https://externals.io/message/109015#109178 cheers, Derick
-- PHP 7.4 Release Manager Host of PHP Internals News: https://phpinternals.news Like Xdebug? Consider supporting me: https://xdebug.org/support https://derickrethans.nl | https://xdebug.org | https://dram.io twitter: @derickr and @xdebug

Christian Schneider

6 years ago
Am 14.04.2020 um 03:10 schrieb Derick Rethans <derick@php.net>:
> On Mon, 13 Apr 2020, Christian Schneider wrote: > >> Am 13.04.2020 um 14:41 schrieb AllenJB <php.lists@allenjb.me.uk>: >>> As no concerns were raised during the discussion period, the RFC is unchanged from that originally posted. >>> >>> Previous discussion threads: >>> * https://externals.io/message/109015 >> >> For the record: I did raise concerns in >> https://externals.io/message/109015#109144 > > You did, but you never followed up on his questions: > https://externals.io/message/109015#109178
I tried to keep the mail traffic down and I didn't think my concern would be discarded because of this. But to answer the question
> Is this not just as disruptive to existing code as changing the default > error handling to warnings?
First of all the RFC talks of changing the default error handling to *exceptions*, not warnings. So yes, I think it should be changed to warnings first. And for that: No, it is not as disrupting as it will write additional error messages to a log file vs. stopping the program execution. - Chris

AllenJB

6 years ago
Hi Chris, Apologies but I assumed your lack of further response or comment meant that my response had cleared up the issues you had with the suggested change. I particularly expected a response in this case since I had questions about (my understanding of) your response - specifically where you talked about "deprecation", which to me, in the context of changes to PHP, means emitting a deprecation notice in some way, and how you saw that would be implemented in the context of this change. The issue was further not raised when I presented the RFC with no open issues or discussion of warnings or deprecation, which I further took to mean that you no longer had any issue. While I get that this can be a busy list and there are occasions where you want to withdraw from further discussion to prevent things going around in circles or getting too astray of the point at hand, as a first time RFC submitter what am I supposed to do when there's no response to my request for clarification? With regards to changing the PDO error mode to warnings first, I would make the following points: PHP has been in the process of upgrading many notices and warnings to thrown errors and introducing new ones throughout the PHP 7 era and going forward into PHP 8, and thrown exceptions or errors is the way I think (particularly newer) users of PHP (will) expect things to happen now. If the default were switched to warnings first, particularly if there's no obvious sign to expect it to change again in the future, new users would learn how to handle PDO errors as warnings (while the rest of PHP primarily uses thrown errors / exceptions), then have to update their code again when it switches to exceptions (this may also apply to the tutorials and guides they follow). (Existing users are much more likely to go about explicitly setting the error mode, or at least knowing how to, but new users are, in my opinion, much more likely to stick and learn with the defaults) An error from the database server is an error, not a warning (MySQL at least has its own concept of warnings, which, from what I've read, the client gets a count of with the initial resultset response but PDO provides no access to) and usually means that the code cannot sensibly continue (this is somewhat mitigated by the fact that you'll usually get subsequent errors attempting to process the resultset - but may not always be the case). Throwing an exception is the safest way to help ensure data integrity, in my opinion. (There will be some cases where people are expecting something may fail and want to ignore that failure - PHP's test suite currently does this when deleting test tables in its PDO factory/constructor - but I would argue that the code should show this expectation) This change is already being made in a major version. It's the sort of change I'd fully expect from a major version. While I understand the desire to reduce the magnitude of breaking changes, I would also argue that keeping their frequency low is just as important (particularly if/when there is no warning that there will be a further change). An alternative would be to force the developer to explictily set the error mode, but I don't see why that should be the case when anything but silent is a good default (with exceptions just being a better default for modern PHP in my opinion). Regards, AllenJB On 14/04/2020 07:52, Christian Schneider wrote:

AllenJB

6 years ago
On 13/04/2020 13:41, AllenJB wrote:
> Hi all, > > I have opened voting on the RFC to change the default PDO error mode: > https://wiki.php.net/rfc/pdo_default_errmode > > As no concerns were raised during the discussion period, the RFC is > unchanged from that originally posted. > > Previous discussion threads: > * https://externals.io/message/109015 > * https://externals.io/message/109398 > > Voting closes on April 27th. > > Regards, > AllenJB >
I've now submitted a pull request for this RFC: https://github.com/php/php-src/pull/5388 Regards, AllenJB

AllenJB

6 years ago
Hi all, Voting has now closed and the RFC has been accepted at 49:2 votes in favor. The PR is available at: https://github.com/php/php-src/pull/5388 If those with servers available could help with checking tests for PDO database specific extensions (or confirming that they don't need updates), that'd be great. (Specifically tests need updating where they don't explicitly set the PDO error mode). I've already done pdo_sqlite and pdo_mysql and pdo_dblib has been checked. Thank you all, AllenJB On 13/04/2020 13:41, AllenJB wrote:

Christoph Becker

6 years ago
On 27.04.2020 at 15:31, AllenJB wrote:
> Voting has now closed and the RFC has been accepted at 49:2 votes in favor. > > The PR is available at: https://github.com/php/php-src/pull/5388 > > If those with servers available could help with checking tests for PDO > database specific extensions (or confirming that they don't need > updates), that'd be great. (Specifically tests need updating where they > don't explicitly set the PDO error mode). I've already done pdo_sqlite > and pdo_mysql and pdo_dblib has been checked.
Thanks for the PR! I have tested pdo_firebird, pdo_mysql, pdo_odbc, pdo_pgsql and pdo_sqlite on Windows; no issues detected.
-- Christoph M. Becker