Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113320
MIME-Version: 1.0
References: <CAGBsUrddzJQu3JtqCdLjyHOBf71gj5LpNKuV=OmYGm0Vd3n9Yw@mail.gmail.com>
 <CAF+90c8GDMs5f0i27n8gGNx_TvwqKH9CqsW8eSgEf+8mGrdyZA@mail.gmail.com>
 <84a86f15-313e-1fb3-eb09-7fd6bbdeb5ce@php.net> <821813eb972cd5dad30a0e10385a115a9a8908a2.camel@schlueters.de>
 <CAGBsUrfF5Xk+ne6fo46EivKN82OQeM=0yAcPe5r2y9-ZgPEYrw@mail.gmail.com>
In-Reply-To: <CAGBsUrfF5Xk+ne6fo46EivKN82OQeM=0yAcPe5r2y9-ZgPEYrw@mail.gmail.com>
Date: Sat, 27 Feb 2021 21:55:40 +0100
Message-ID: <CAEPJdu=j86qixL-2okuvE9T7eCgea-zOrCd5M1AnqETCoLo-FQ@mail.gmail.com>
To: Kamil Tekiela <tekiela246@gmail.com>
Cc: =?UTF-8?Q?Johannes_Schl=C3=BCter?= <johannes@schlueters.de>, 
	Sebastian Bergmann <sebastian@php.net>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000260a2305bc579cc2"
Subject: Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode
From: kontakt@beberlei.de (Benjamin Eberlei)

--000000000000260a2305bc579cc2
Content-Type: text/plain; charset="UTF-8"

On Sat, Feb 27, 2021 at 5:24 PM Kamil Tekiela <tekiela246@gmail.com> wrote:

> >
> > The issue is, as said, that this only happens in an error situation.
> > Thus if users only test "does my site still work after update?" (what
> > many do ...) won't notice this, till something fails, which would have
> > been caught nice beforehand.
> >
>
> I don't see why this would be such an issue. If the code fails for any
> reason then an error is produced. The error might be surprising, but it is
> not something that would cause problems.
>

I think you are very wrong about this, there could be perfectly valid code
doing:

if (!$conn->query("select 1") && $conn->errno == 2006) { // mysql gone away
    $conn = new mysqli(....);
}

With your proposed change the query would throw an exception instead,
potentially breaking follow up code that is required to run.

This is the kind of code you have that only gets triggered every 100000
times and that is very hard to write a real test for. If the test here uses
a mock, then you wont catch the behavior change.


> This change fixes more problems than it creates. I believe it is still
> worthy to put it in 8.1.  Every PHP release contains backwards-incompatible
> changes. In comparison to other BC changes PHP has put in minor releases
> this one has a very easy fix. You don't have to scour the code looking for
> places to fix.
>

There are projects out there that do a mysqli_connect on *every* script.
You underestimate the impact of this break.

PHP can't improve if we have to think of people who upgrade without reading
> the release notes. We also shouldn't be held back by outdated tutorials. In
> fact, this will help people who still use such tutorials. This isn't a
> major change that would affect everyone on the planet. This will only
> affect people who still use mysqli and didn't set the error reporting mode
> before upgrading.
>

I am sorry, but to me that feels a bit like you want to take the easy way
out..

As a counterexample, Nikita made a very good RFC for 8.1 / 9.0 deprecating
and changing the nullable to internal functions behavior:
https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg

This RFC goes above and beyond to provide a migration path, deprecation
messages and such.

Yes it has a higher potential break impact, but i don't expect this change
to wait until 9.0 either.

I would wish your RFC would throw a deprecation message in 8.1 that you
cannot make a query without setting the reporting level before by calling
mysqil_report() explicitly. Then in 8.2 we could change the behavior.


> I understand your worries but I don't think this is a good enough reason to
> not go ahead with this change.
>
> Kind regards,
> Kamil
>

--000000000000260a2305bc579cc2--