Skip to content

Remove support for libmysql-client from mysqli test suite #9652

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

Closed
wants to merge 3 commits into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Oct 1, 2022

Since mysqli can no longer be built against libmysql-client, there is no longer the need to distinguish.


By the way, do we really still support MySQL < 5.0?

Since mysqli can no longer be built against libmysql-client, there is
no longer the need to distinguish.
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@iluuu1994
Copy link
Member

By the way, do we really still support MySQL < 5.0?

It's so old I could find a reference to when support was dropped, but even 5.0 was EOL a decade ago. So definitely not.

@andypost
Copy link
Contributor

andypost commented Oct 3, 2022

if mysqli depends mysqlnd why that not pointed in config.m4 file? moreover mysqlnd has it names as config9.m4 which means it will be loaded later mysqli

@cmb69
Copy link
Member Author

cmb69 commented Oct 4, 2022

It's so old I could find a reference to when support was dropped, but even 5.0 was EOL a decade ago. So definitely not.

The documentation disagrees:

The mysqli extension allows you to access the functionality provided by MySQL 4.1 and above.

We should do something about this. :)

if mysqli depends mysqlnd why that not pointed in config.m4 file? moreover mysqlnd has it names as config9.m4 which means it will be loaded later mysqli

I guess that should be addressed as well.

@@ -16,22 +16,8 @@ require_once('skipifconnectfailure.inc');
var_dump($mysql->ping());

$ret = $mysql->kill($mysql->thread_id);
if ($IS_MYSQLND) {
Copy link
Member

@kamil-tekiela kamil-tekiela Oct 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please drop the $version variable altogether from this file.

@kamil-tekiela
Copy link
Member

The documentation disagrees:

The mysqli extension allows you to access the functionality provided by MySQL 4.1 and above.

I don't know what changes were made between MySQL 4 and 5 that would make it incompatible with mysqli. The fact is that MySQL 4.1 is the lowest possible version for ext/mysqli, but I don't know if we actively test support for this version. I think the tests are not even suitable for execution with MySQL 4. We certainly cannot guarantee that ext/mysqli still works with MySQL 4.

I can't imagine any sane project running MySQL < 5.6, and even that version has lost official Oracle support last year. But the truth is that there are a lot of insane developers who might still be using MySQL 5.5 or older in production.

I think it's safe to assume that ext/mysqli will work fine with MySQL 5.5 or newer, but we don't officially have a supported version bracket. It might work without any issues with MySQL 4 too. Things get more complicated when we consider MariaDB, Percona or other MySQL-like databases. Personally, I am still stuck with MariaDB 10.3/10.4 and I haven't tested 10.6 yet, not to mention 10.9, 10.10, or 10.11. I hope that PHP can still work with all these newer versions, but I can't guarantee it.

Why do you ask? I know we have some code in mysqlnd that checks for MySQL bugs in 5.1, but other than that we shouldn't have version-specific code.

@cmb69
Copy link
Member Author

cmb69 commented Oct 4, 2022

I know we have some code in mysqlnd that checks for MySQL bugs in 5.1, but other than that we shouldn't have version-specific code.

There are a lot of mysqli_get_server_version() checks in the test suite (more than 50), and many check for very old versions (5.0.x). We could simplify the tests a bit, if we officially drop support for such old versions. And of course it would be nice for users to be able to check the documentation and to know what to expect.

@kamil-tekiela
Copy link
Member

I think we can remove these checks from tests anyway. We haven't been paying a lot of attention to them in recent years and if you tried to execute the test suite on MySQL 4, you would definitely get a lot of failures. We are not testing ext/mysqli against old MySQL versions (correct me if I am wrong) and I don't expect we will be doing such tests in the future.

@cmb69 cmb69 closed this in 62d393b Oct 6, 2022
@cmb69
Copy link
Member Author

cmb69 commented Oct 6, 2022

I think we can remove these checks from tests anyway.

Okay, I'll come up with another PR soonish (PDO_MySQL should also be checked accordingly).

@cmb69 cmb69 deleted the cmb/is_mysqlnd branch October 6, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants