Check liveness in PDO_ODBC #6805
Conversation
PDO drivers allow for it, and procedural ODBC has its own facility for it, but PDO_ODBC doesn't. If a connection is severed (for example, on IBM i, ending a databse job, or killing the network connection elsewhere), a persistent connection could get stuck. This adapts the procedural ODBC code to PDO for handling connection liveness, so PDO can reconnect if needed. A discussion about the method to check liveness is linked; this might not be the best method, but it's what procedural ODBC uses, so it's consistent.
|
FWIW, I also have backport patches for 7.4 and 8.0; just related to |
ext/pdo_odbc/odbc_driver.c
Outdated
|
|
||
| /* | ||
| * XXX: Should we be using SQL_ATTR_CONNECTION_DEAD? Procedural ODBC | ||
| * uses this check, but it might be problematic per #20298 |
There was a problem hiding this comment.
Yes, I think we should check for SQL_ATTR_CONNECTION_DEAD. ext/odbc likely doesn't do this, because SQLGetConnectAttr () is ODBC 3.0 and the ODBC 2.0 SQLGetConnectOption() doesn't support this attribute, but ext/odbc is still supposed to work with ODBC 2.0 (something we probably should have changed years ago). ext/pdo_odbc requires ODBC 3.0 anyway.
|
Switching it the connection attribute, though I do notice that's actually an ODBC 3.5 thing, and MS offers an additional Not as throughly tested; I'll see if the db2i ODBC driver works, at least. |
ext/pdo_odbc/odbc_driver.c
Outdated
| */ | ||
| ret = SQLGetInfo(H->dbc, SQL_DATA_SOURCE_READ_ONLY, d_name, | ||
| sizeof(d_name), &len); | ||
| /* ODBC 3.5; procedural ODBC uses SQLGetInfo read only instead */ |
There was a problem hiding this comment.
Oh, you're right, SQL_ATTR_CONNECTION_DEAD is ODBC 3.5. We may need a fallback for ODBC 3.0 (maybe just not checking the liveliness there is okay).
There was a problem hiding this comment.
Yeah, I'm not having much luck with the Db2i driver on Linux; it reconnects, but dead returns both SQL_SUCCESS and dead == 0 (SQL_CD_FALSE). (And a segfault at the end, but I can only assume that might be my fault for running bleeding-edge Fedora.) I can try it on i directly too, but it's the same codebase on both.
If drivers can't report their dead status reliably, maybe using SQLGetInfo only or to verify if the driver is correct would work.
Not sure which version this should target; it's somewhere between a bugfix and a new feature.
Not sure if we should support that; the recommended PDO driver for SQLServer is http://pecl.php.net/pdo_sqlsrv anyway. |
|
OK, it's crashing on debian oldstable without my patches applied, so I'm chalking that up to a db2i driver bug on Linux. 🤨 I'll see how the dead attr works on i. If it's as unreliable... |
|
Come to think of it, it might even be a leak in PHP now that I have |
|
So far:
|
|
The Linux issue is probably separate and filed as #80909. |
Using the dead attr in the liveness check is ODBC >=3.5 and doesn't seem to work reliably across drivers; for example, the IBM i Access driver seems to have issues with it.
|
I rolled back the dead attr check; I don't think it's reliable or well supported enough to rely on. Maybe with a fallback, but then you'd add more possible round trips and increase the cost of persistent connection checks... |
|
CI failures were unrelated. |
|
This looks reasonable to me. @cmb69 Any further comments? |
PDO drivers allow for it, and procedural ODBC has its own facility for it, but PDO_ODBC doesn't. If a connection is severed (for example, on IBM i, ending a database job, or killing the network connection elsewhere), a persistent connection could get stuck. This adapts the procedural ODBC code to PDO for handling connection liveness, so PDO can reconnect if needed.
A discussion about the method to check liveness is linked; this might not be the best method, but it's what procedural ODBC uses, so it's consistent.