Hi, can I get a review of this PR please.

MariaDB versioning created a mess with regarding testing features based on version.

Well, testing for featured based on version is generally not the best solution, in my opinion. Your changes look plausible to me, but I'm not really an mysqli expert, and wouldn't be able to test with any libmysql-client (I'm working on Windows).

Anyhow, I wonder whether that bug hasn't been fixed already; while Andrey closed the ticket as WONTFIX, and said he'd going to revert my commit, he apparently did not. :/

Thanks for the endorsement. There is a libmysql-client for Windows.

I tested it a bit more and it looks still to be fixed without this (I've asked the original reporter to explain https://jira.mariadb.org/browse/MDEV-23141 to check). Maybe its in mysqli rather than mysqlnd? The 5.5.5 hack is still in the mariadb code base. The only other php ext uses of mysql_get_server_version are against a 5.0 version.

Well at least this alternate code is here if a revert gets decided upon.

Thanks for the link to the Windows libmysql-client; the other part of the problem is, however, that there isn't any support for building against libmysql-client for Windows. I may have a look at this sometime.

Well at least this alternate code is here if a revert gets decided upon.

I'm not happy with the version hack in PHP myself. And even without the need for any hack, version comparisons won't really work anymore due to having different systems with individual versioning now.

Anyhow, @andreyhristov, what do you think? Should we still remove the MariaDB version detection hack? Or keep it, but apply this PR anyway?

Hi,
I never really liked the hack and was against it being incorporated but it was incorporated. mysqlnd is a library for the MySQL C/S protocol and the reference implementation is in MySQL / libmysql. If MariaDB needs to hack this, they are free to write a mysqlnd plugin that make the needed changes and distribute it from PECL or some other channel.

I don't like that version hack either, and I'm not against moving it to a PECL hosted mysqlnd plugin. However, even if we do so, version checks can't really work in the long run, since MariaDB apparently does not conform to MySQL versioning, so we either should replace the version checks with more general feature detection (or in this case, as done in this PR), or we should document that mysqlnd doesn't fully support MariaDB.

Hi,

I regret the revisited trauma of mentioning this versioning trauma again. I don't think anyone saw it as a good solution, only one needed for MariaDB to maintain/claim some replication compatibility with MySQL.

I've managed to reproduce the original problem identified by bug reporter in MDEV-23141 by compiling php against the minimal tarball of mysql with ./configure --enable-debug --with-mysqli=/usr/local/mysql-8.0.21-linux-glibc2.17-x86_64-minimal/bin/mysql_config --with-pdo-mysql=/usr/local/mysql-8.0.21-linux-glibc2.17-x86_64-minimal/ --with-mysql-sock=/tmp/s.sock) (along with a few php mysqli patches needed avoid compiling against the server interfaces(I'll clean these up and submit them separately)).

As such, only the mysqli section of this PR is needed to expose the transaction capabilities of MariaDB and the mysqlnd changes are unnecessary (though they look good from a consistency point of view and are covered by existing test cases, you're call however, I'm happy to remove the mysqlnd changes).

To ensure that no regressions are caused by MariaDB server against the current php mysqli/mysqlnd drivers I've started this project to integrate into the mariadb.org bb system such that server incompatibilities get detected pre-release and the php project doesn't get asked to account for MariaDB differences. Regardless if we need to produce our own plugin/PECL maintaining existing compatibility for our common user base is a good first goal.

Please hold off on merging this for now, I'm pretty sure its right but won't know until mysqli corrections have been made more reliable (SEGV in mysql_server_end that needs to be corrected/avoided).

Or maybe MariaDB should stop advertising as drop-in replacement :) Andrey

I totally agree and the removal of that now obsolete, incorrect, byline has been going on for a while. Still a few places to remove it from however.

with #6126 merged, thanks @nikic, there's now a more functional mysqli native client implementation to work this on.

After a rebase just pushed, testing against MySQL-5.6, 5.7 and 8.0 do pass the mysqli_begin_transaction.phpt test exercised by this change - https://travis-ci.org/github/grooverdan/php-src/builds/728129550

The mysqlnd also passes this test, though as mentioned earlier, the mysqlnd changes in this PR aren't strictly necessary, though it does add to a consistent implementation.

With MySQL-5.5 ending its extended support 18 months ago, those that run supportable systems will already be running on a server that supports TRANS_START_READ_{ONLY|WRITE}. Those running older versions won't be intentionally hitting this error message. So we're reducing a server version check from the processing chain for the majority of users.

Merged as 740f0f6.

Thanks @nikic