Personally, given that both of these versions are EOL, this feels
acceptable, but I’m curious what others think. Notably, if we were to
implement configuration for this we’d likely need to introduce yet
another INI variable; I have an impression that PHP maintainers are
somewhat skeptical of adding new INI settings.
I think it’s ok to drop support of EOL versions. But I also think it’s possible to support both new and ancient mysql even without any runtime options (ini setting, PDO constant et al). First try to send COM_RESET_CONNECTION and if server return error, try COM_CHANGE_USER. Result of this check may be cached for improved performance, but not strictly necessary. This at least won’t break anyone.
But if you are feel strongly about completely dropping COM_CHANGE_USER, then some deprecation period should exist. Users should see at least E_DEPRECATED errors in their logs before day X. I know about changelog page and NEWS file, but among all the PHP programmers I know, no one reads them.
Secondly, I’m curious what people think about implementing this in
both PDO andmysqlnd. In PDO, we can replace the liveness check
with this command, which would act as both a liveness check and a
reset for no additional round-trip cost.
It’s ok to replace liveness check with reset, but I think you choose wrong place for implementation. You changed the pdo_mysql_check_liveness function and now it not just “liveness check”, which leads to a discrepancy with its name and also liveness functions from other PDO drivers. I’d rather implement in on PDO level. Something like:
// pdo_dbh.c:420
/* is the connection still alive ? */
- if (pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh)) {
+ if (!(pdbh->methods->reset && (pdbh->methods->reset)(pdbh)) && pdbh->methods->check_liveness && FAILURE == (pdbh->methods->check_liveness)(pdbh)) {
/* nope... need to kill it */
pdbh->refcount--;
zend_list_close(le);
pdbh = NULL;
}
See also the sluggish discussion about PDO: https://externals.io/message/129740