Add support for duplicate named query parameters to the mysqli driver

[nikosdion]

Summary of Changes

Added support for duplicate named query parameters to the mysqli and sqlsrv drivers. What I mean is something like this:

SELECT * FROM dbtest WHERE `title` LIKE :search OR `description` LIKE :search

Note how :search is used twice in the above query.

This had always worked with all PDO-based drivers: mysql, pgsql, and sqlite. This feature was added to PDO back in PHP 5.2.1.

The two non-PDO drivers use userland code to emulate prepared statements, but the userland code didn't support multiple uses of the same named parameter.

Testing Instructions

Run the tests contained in this PR before and after applying the patch for each of the mysql, mysqli, sqlite, sqlsrv, and pgsql database drivers.

Before the PR changes are applied:

• mysql: Fails with an exception about mismatched number of arguments
• mysqli: Works
• pgsql: Works
• sqlite: Works
• sqlsrv: Fails with an exception about mismatched number of arguments

After the PR changes are applied:

• mysql: Works
• mysqli: Works
• pgsql: Works
• sqlite: Works
• sqlsrv: Works

Documentation Changes Required

None.

Notes

I have inadvertently been using this feature since Joomla! 4.0 beta 2 was released with the mysql driver. It was only when I received bug reports from users using the mysqli driver I figured out something is wrong, which is how I ended up with this PR.

PDO has supported multiple uses of the same named parameter since PHP 5.2.1. However, because many people remembered the prior situation you will see a lot of wrong information online stating that PDO does not support this feature.

I have tested the mysql and mysqli drivers using MySQL 8 installed directly on my host OS. I tested sqlite directly, using an in-memory database as per the default configuration. I tested pgsql against a Dockerized PostgreSQL server, using the postgresql:latest image. I tested sqlsrv against a Dockerized Microsoft SQL Server Express 2019 installation -- but I had to change \Joomla\Database\Sqlsrv\SqlsrvDriver::connect to set Encrypt to false, since the Dockerized server does not have a valid TLS certificate. All my tests have been against PHP 8.3 using the PHPUnit version pulled into the vendor folder on composer install to keep things clean and simple.

The current MS SQL Server driver uses the sqlsrv PHP extension. If it used the pdo_sqlsrv extension, available since PHP 7.1, it wouldn't need a custom SqlsrvStatement class, nor would it have a problem with this feature to begin with. Maybe it's worth adding a pdosqlsrv driver?

[HLeithner]

Our mysql implementation of named prepared statement matches the support by other drivers (especially postgres). Does this also work in the other supported drivers?

edit: I did some quick search but didn't found it in the php documentation only found a 7 year old SE which says no. Has this been changed or are the answers wrong?

[nikosdion]

I had been inadvertently using multiples of same-named parameters with the mysql driver since Joomla was at 4.0 beta 2, so I'm sure they work based on my lived experience. I started getting bug reports from the minority of users on mysqli which is how I ended up in this here rabbithole 🕳️🐇

You do not necessarily have feature parity with other PDO providers as you're using PDO::ATTR_EMULATE_PREPARES which is only available on MySQL (among the DB servers you support). No idea why, not explained in the commit message either, and Michael who wrote the code has quit and gone no-contact with the project a long time ago.

I can check if Postgres and SQLite also support multiple substitutions. Give me a couple of hours, I'll get back to you on that.

[nikosdion]

I did a quick check, duplicate keys work perfectly fine with SQLite. Interesting. I guess the SF answer was probably outdated or wrong, as you said.

Once I get some time later today I'll test with Postgres and, if I am masochistic enough, MS SQL Server.

[HLeithner]

j4i named prepared statements for mysqli has been written by me, michael did the prepared statement stuff without named parameters.

But that doesn't matter, if the same thing works for postgres, I'm sure we get a mssql server test too for this.

[nikosdion]

Sorry, I was unclear in my wording. Michael committed the mysql driver in 2018 where he had the PDO::ATTR_EMULATE_PREPARES flag without explanation as to why.

I will try to install PostgreSQL and MS SQL Server later, and port the tests to be certain. As far as I can see trawling through old PHP issues, multiple instances of the same named parameter in PDO prepared statements have been supported since PHP 5.2.1.

[nikosdion]

I tested with all available drivers.

As I suspected, all PDO drivers worked perfectly out of the box. Support for this feature was added in PHP 5.2.1. The information claiming otherwise appears to be people remembering the situation before PHP 5.2.1, repeating their memory as a fact, without having checked if it's still true 🤷‍♂️

MS SQL Server does not use PDO, so I had to modify its statement class as well.

[mbabker]

Michael who wrote the code has quit and gone no-contact

Hi, I'm the no-contact guy 😆

Believe it or not, the PDO-based MySQL driver has always used PDO::ATTR_EMULATE_PREPARES, you've gotta dig all the way back to https://github.com/joomla/joomla-framework/pull/218 for when that was introduced.

Everything I did years ago was modeled around what the doctrine/dbal API looked like in this part of the database integration since what was being worked on was mostly a solved problem for them. There's no real rhyme or reason beyond "these folks who are smarter than me came up with this solution so it seems like a good idea to borrow from them", and I tried to not change much from what was already there in what I was working on.

It looks like the only thing I did there was update that line to use an internal method for setting attributes instead of calling the PDO connection directly, and I don't think that config has been challenged at all since the driver's introduction. So you're probably looking at a case of "this worked fine when implemented in 2013, and nobody's touched it since".

[nikosdion]

@mbabker I thought you were no longer commenting on Joomla! repos 😄 Yes, it makes sense copying Doctrine, they have a far higher installation base than Joomla! ever will and they know what they're doing.

Well, intentional or not, PDO drivers work great thanks to this one little trick. The mysqli and sqlsrv drivers, not so much. If someone is really interested in fixing it I have submitted the code which addresses the issue. It wasn't too hard to get there, once you know what you're looking for. And I can totally see why nobody looked at it before; it's not exactly a common use case.

[mbabker]

I thought you were no longer commenting on Joomla! repos 😄

I couldn't convince anyone to block me on this org so there are still rare occasions that I might be convinced to drop a nugget of knowledge (or just flat out call something out for the bad decision it is, I have one of those issues too 😅)

[wilsonge]

Thanks for the fix! Can't tag it right now as on a phone but will try and sort it out when im home later if someone doesn't step in before then

[HLeithner]

thanks george but looking at the test would be helpful... @nikosdion are you willing to fix the tests? CS, Postgres nd MSSQL? https://ci.joomla.org/joomla-framework/database/1459/1/3