Nothing Special   »   [go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

MariaDB version prefix 5.5.5- is not stripped #7972

Closed
mvorisek opened this issue Jan 11, 2022 · 8 comments
Closed

MariaDB version prefix 5.5.5- is not stripped #7972

mvorisek opened this issue Jan 11, 2022 · 8 comments
Assignees
@cmb69

Comments

@mvorisek
Copy link
Contributor

Description

The version from SQL (from select version()) should match the version reported by php mysqli/pdo_mysql drivers.

The following code:

var_dump(...->expr('select version()')->getOne()); // raw version from SQL
var_dump(...->getWrappedConnection()->getServerVersion()); // version from DBAL/mysqli

Resulted in this output:

string(35) "10.6.5-MariaDB-1:10.6.5+maria~focal"
string(41) "5.5.5-10.6.5-MariaDB-1:10.6.5+maria~focal"

But I expected this output instead:

string(35) "10.6.5-MariaDB-1:10.6.5+maria~focal"
string(41) "10.6.5-MariaDB-1:10.6.5+maria~focal"

PHP Version

8.0

Operating System

any
@cmb69
Copy link
Member
cmb69 commented Jan 11, 2022

This "most unfortunate" MariaDB version prefix broke compatibility with transactions (and likely more); therefore we fixed https://bugs.php.net/78179 by stripping that prefix. Anyway, I'm not sure what you're requesting here. Can you please clarify whether you want the prefix stripped, or retained? In the former case, that doesn't look like a php-src issue at all.

@mvorisek
Copy link
Contributor Author

in

php-src/ext/mysqlnd/mysqlnd_connection.c

Line 1281 in 72cdb0a

#define MARIA_DB_VERSION_HACK_PREFIX "5.5.5-"
there is a fix to parse the version correctly, however

mysqli::get_server_info method still returns the unfixed/prefixed version because of https://jira.mariadb.org/browse/MDEV-4088 protocol fix for replication

the version returned by mysqli::get_server_info should be fixed and should match version returned by select version()

@kamil-tekiela
Copy link
Member

We can't do any stripping. We follow what the C API does in this case, which is to return the string verbatim. Any stripping would be against the specification. If the value returned by mysqli_get_server_info() needs to be stripped of any characters, it can be done in userland. I don't see any mention in any documentation that the value returned by mysql_get_server_info should match the one returned by SQL select version().

If the issue is that MariaDB is prefixing this value in the first place, then this should be reported to MariaDB.

I believe the current behaviour of PHP/mysqlnd is correct and I don't see any buggy behaviour.

@mvorisek
Copy link
Contributor Author

mysqli_get_server_version() is fixed (in php-src) - is there any support to get the numerical version also from PDO?

We follow what the C API does in this case, which is to return the string verbatim.

The stripping is actually official for MariaDB client, see one related MariaDB issue https://jira.mariadb.org/browse/MDEV-4575 . As the prefixing is known and introduced only as a fix for replication protocol, it is safe to be removed also for mysqlnd.

@cmb69 cmb69 self-assigned this Jan 11, 2022
@cmb69
Copy link
Member
cmb69 commented Jan 18, 2022 
$mysql = new mysqli($host, $user, $password, $db);
$res = $mysql->query("select version()");
var_dump(
    $mysql->server_info,
    $res->fetch_array()[0],
    $mysql->server_version,
    mysqli_get_server_version($mysql),
);

outputs for me

string(21) "5.5.5-10.5.13-MariaDB"
string(15) "10.5.13-MariaDB"
int(100513)
int(100513)

This inconsistency is certainly a bug.

@kamil-tekiela
Copy link
Member

This inconsistency is certainly a bug.

Do you mean a bug in mysqlnd? How so? This is exactly the information that mysqlnd receives from MariaDB.

@cmb69
Copy link
Member
cmb69 commented Jan 18, 2022

This is exactly the information that mysqlnd receives from MariaDB.

No. We already strip the version prefix for ::$server_info and ::$server_version, but not when setting the conn->server_version in MYSQLND_METHOD(mysqlnd_command, handshake). Either we strip it everywhere, or never.

cmb69 referenced this issue in cmb69/php-src Jan 18, 2022
@cmb69
Fix GH-7932: MariaDB version prefix not always stripped
3306f47 
For consistency with `mysqlnd_conn_data#get_server_version`, we strip
the MariaDB version prefix also in `mysqlnd_command#handshake`.
@cmb69
Copy link
Member
cmb69 commented Jan 18, 2022

After some further discussion on #7963, we decided to keep the behavior as is, but to document it.

@cmb69 cmb69 transferred this issue from another repository Jan 18, 2022
@kamil-tekiela kamil-tekiela transferred this issue from php/doc-en Jan 19, 2022
pento pushed a commit to WordPress/wordpress-develop that referenced this issue Aug 22, 2022
@SergeyBiryukov
Tests: Correct MariaDB version check in database charset tests.
3e74023 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to the `Tests_DB_Charset` class to check for the correct version.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]

Follow-up to [53918].

Fixes #53623.

git-svn-id: https://develop.svn.wordpress.org/trunk@53919 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this issue Aug 22, 2022
@SergeyBiryukov
Tests: Correct MariaDB version check in database charset tests.
2699a5f 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to the `Tests_DB_Charset` class to check for the correct version.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]

Follow-up to [53918].

Fixes #53623.
Built from https://develop.svn.wordpress.org/trunk@53919


git-svn-id: http://core.svn.wordpress.org/trunk@53478 1a063a9b-81f0-0310-95a4-ce76da25c4cd
gMagicScott pushed a commit to gMagicScott/core.wordpress-mirror that referenced this issue Aug 22, 2022
@SergeyBiryukov
Tests: Correct MariaDB version check in database charset tests.
8bcc75d 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to the `Tests_DB_Charset` class to check for the correct version.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]

Follow-up to [53918].

Fixes #53623.
Built from https://develop.svn.wordpress.org/trunk@53919


git-svn-id: https://core.svn.wordpress.org/trunk@53478 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pbearne pushed a commit to pbearne/wordpress-develop that referenced this issue Sep 9, 2022
@SergeyBiryukov @pbearne
Tests: Correct MariaDB version check in database charset tests.
011961b 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to the `Tests_DB_Charset` class to check for the correct version.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]

Follow-up to [53918].

Fixes #53623.

git-svn-id: https://develop.svn.wordpress.org/trunk@53919 602fd350-edb4-49c9-b593-d223f7449a82
whereiscodedude pushed a commit to whereiscodedude/wpss that referenced this issue Sep 18, 2022
Tests: Correct MariaDB version check in database charset tests.
8455e59 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to the `Tests_DB_Charset` class to check for the correct version.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]

Follow-up to [53918].

Fixes #53623.
Built from https://develop.svn.wordpress.org/trunk@53919
markjaquith pushed a commit to markjaquith/WordPress that referenced this issue Oct 4, 2022
@SergeyBiryukov
Database: Correct MariaDB version check in wpdb::has_cap().
2ede199 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to `wpdb::has_cap()` to check for the correct MariaDB version.

This resolves an issue where the `utf8mb4_unicode_520_ci` collation, which is available in MariaDB since version 10.2, was previously not detected correctly.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]
* [https://mariadb.com/docs/reference/mdb/collations/utf8mb4_unicode_520_ci/ MariaDB Documentation: utf8mb4_unicode_520_ci]

Follow-up to [37523], [53919].

Props jamieburchell, SergeyBiryukov.
Fixes #54841.
Built from https://develop.svn.wordpress.org/trunk@54384


git-svn-id: http://core.svn.wordpress.org/trunk@53943 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this issue Oct 4, 2022
@SergeyBiryukov
Database: Correct MariaDB version check in wpdb::has_cap().
1aa4c36 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to `wpdb::has_cap()` to check for the correct MariaDB version.

This resolves an issue where the `utf8mb4_unicode_520_ci` collation, which is available in MariaDB since version 10.2, was previously not detected correctly.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]
* [https://mariadb.com/docs/reference/mdb/collations/utf8mb4_unicode_520_ci/ MariaDB Documentation: utf8mb4_unicode_520_ci]

Follow-up to [37523], [53919].

Props jamieburchell, SergeyBiryukov.
Fixes #54841.
Built from https://develop.svn.wordpress.org/trunk@54384


git-svn-id: https://core.svn.wordpress.org/trunk@53943 1a063a9b-81f0-0310-95a4-ce76da25c4cd
ootwch pushed a commit to ootwch/wordpress-develop that referenced this issue Nov 4, 2022
@SergeyBiryukov
Tests: Correct MariaDB version check in database charset tests.
4f1f27d 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to the `Tests_DB_Charset` class to check for the correct version.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]

Follow-up to [53918].

Fixes #53623.

git-svn-id: https://develop.svn.wordpress.org/trunk@53919 602fd350-edb4-49c9-b593-d223f7449a82
ootwch pushed a commit to ootwch/wordpress-develop that referenced this issue Nov 4, 2022
@SergeyBiryukov
Database: Correct MariaDB version check in wpdb::has_cap().
3ea40bc 
MariaDB version is reported differently between PHP versions:
* PHP 8.0.16 or later: `10.6.8-MariaDB`
* PHP 8.0.15 or earlier: `5.5.5-10.6.8-MariaDB`

The latter includes PHP 7.4.x and PHP 5.6.x as well, where the version is also reported with the `5.5.5-` prefix.

This commit makes an adjustment to `wpdb::has_cap()` to check for the correct MariaDB version.

This resolves an issue where the `utf8mb4_unicode_520_ci` collation, which is available in MariaDB since version 10.2, was previously not detected correctly.

References:
* [php/php-src#7972 php-src: #7972: MariaDB version prefix 5.5.5- is not stripped]
* [php/php-src#7963 php-src: PR #7963 Fix GH-7932: MariaDB version prefix not always stripped]
* [https://mariadb.com/docs/reference/mdb/collations/utf8mb4_unicode_520_ci/ MariaDB Documentation: utf8mb4_unicode_520_ci]

Follow-up to [37523], [53919].

Props jamieburchell, SergeyBiryukov.
Fixes #54841.

git-svn-id: https://develop.svn.wordpress.org/trunk@54384 602fd350-edb4-49c9-b593-d223f7449a82
@jduke-halls jduke-halls mentioned this issue Sep 11, 2023
Closed
@Jolg42 Jolg42 mentioned this issue Feb 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cmb69 cmb69

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mvorisek @cmb69 @kamil-tekiela and others