Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:112767
MIME-Version: 1.0
References: <CAGBsUrei=az9TERLhZukybY1hH1wVq6adEc=3UHrCcjHdtc5QQ@mail.gmail.com>
In-Reply-To: <CAGBsUrei=az9TERLhZukybY1hH1wVq6adEc=3UHrCcjHdtc5QQ@mail.gmail.com>
Date: Tue, 5 Jan 2021 17:45:33 +0100
Message-ID: <CAF+90c-JL_9iPkmZ1dj01NyRSGsoZ5d-M6UgGfO7PMxnd5cbrQ@mail.gmail.com>
To: Kamil Tekiela <tekiela246@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000067a6a005b829f0a3"
Subject: Re: [PHP-DEV] Mysqli improvements
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000067a6a005b829f0a3
Content-Type: text/plain; charset="UTF-8"

On Wed, Dec 30, 2020 at 7:33 PM Kamil Tekiela <tekiela246@gmail.com> wrote:

> Hi Internals,
>
> I would like to start a discussion about possible improvements to the
> mysqli API. I have written an RFC which explains all my ideas.
>
> https://wiki.php.net/rfc/improve_mysqli
>
> As the RFC is nothing more than a concept at the moment I am looking
> for some feedback. I attempted to implement some of the changes myself
> but due to my limited experience with C and PHP internals I didn't get
> far. I would appreciate if some volunteer would like to help me to
> implement the changes once they are ironed out.
>
> I understand that the RFC will need to be split up before voting.
>
> Kind regards,
> Kamil Tekiela
>

This RFC appears to cover mostly unrelated topics:

Enable exception error reporting by default: Yes, we should definitely do
this. Especially as we did the corresponding PDO change. I think people
often don't fully appreciate that in mysqli errors can occur all the way
down -- it's not enough to check whether your prepare and execute succeed,
you might still get an error during an individual fetch...

Bind in execute: Sounds like a good idea. I also glanced over your pull
request, and it looks like supporting this is very straightforward.

3onnect_error property: While these probably should have been static
methods (I don't think they can really be static properties from a
technical perspective), I'm not sure this is a problem worth addressing. If
exception mode is enabled by default, why would one ever want to use these
properties?

Under `''new mysqli'' and ''mysqli_connect()'' are not true aliases`, your
point 3 seems like an unneeded rant. When the manual lists a constructor
and a procedural equivalent as aliases, the relationship is always the
same. Similar to how the documentation can list a property as an alias of a
function, even though they aren't technically the same -- duh, who would've
thought.

Regarding your proposal to clean up the construction mess:
 * Removing mysqli::init() sounds reasonable to me. I'm having a really
hard time coming up with a case where it would even do something sensible.
Possibly if you do $mysqli->close() and then want to re-initialize the
object? Very dubious...
 * Aligning mysqli_connect() and new mysqli() without arguments. I don't
think we should do this. There are two ways to establish a connection:
mysqli_init() + mysqli_real_connect(), or just standalone mysqli_connect().
Allowing mysqli_connect() + mysqli_real_connect() as well doesn't seem like
an improvement to me. Really, the "new mysqli()" exception without args
should not have existed, instead a static "mysqli::init()" method should
take that role. Then "new mysqli" would really be equivalent to
"mysqli_connect" and "mysqli::init()" would really be equivalent to
"mysqli_init()." I think that rather than removing mysqli->init(), it may
make more sense to convert it into a static method. We probably can't
remove the "new mysqli" zero args exception for BC reasons, but this would
at least make everything else sane and nicely symmetric between procedural
and OO style.
 * Aliasing connect and real_connect. This seems okay to me. Again, the
intended usage here is "connect" or "init + real_connect", but I don't see
an immediate reason why making this "init + connect" wouldn't work.
(Allowing the end-user to specify client capability flags seems pretty
dubious, but that's an unrelated issue...)
 * Deprecating mysqli_set_opt. IMHO the mysqli_set_opt name is better than
the mysqli_options one. We only have the latter because it happens to be
called mysql_options in the underlying C API. I wouldn't deprecate this
alias.

libmysqlclient support: This is a tricky question. I have spent some time
improving PDO MySQL compatibility with libmysqlclient and our test suite
now passes with it. However, the state of mysqli is much worse. We have
many failing tests, and also many functions/methods that are only
implemented if mysqlnd is used. A possibility here is to drop support for
libmysqlclient in mysqli and only support it in PDO, where we have a much
smaller API surface to deal with.

Regards,
Nikita

--00000000000067a6a005b829f0a3--