Open NattyNarwhal opened 1 year ago
Mea culpa (though there's still an issue); I had assumed their application was using PDO_ODBC, when it's actually using procedural ODBC. The PDO_ODBC case would have to fail now that I read it clearly, because it would break out of the while (rc == SQL_NEED_DATA)
loop. The error reporting wouldn't be specific for the function, but it would show the right error.
However, procedural ODBC does not check for errors in SQLPutData
, so it would fail in the next iteration of the loop on SQLParamData
. In fact, the error reporting for that chunk of code seems pretty sketchy in general.
I think I'll get the user to try PDO_ODBC and see if that works better.
My mea culpa has a mea culpa - it is PDO_ODBC that the user was running. However, I think both extensions suffer this issue from reading their source, but it's not clear to me why PDO_ODBC is also suffering.
I stepped through PDO_ODBC in gdb and found out:
SQLPutData
- specifically, character conversion to match the column's type failed. (That was worked around by a driver connection string option, though it doesn't solve this issue).PDO_ODBC_CONV_NOT_REQUIRED
case that gets used. SQLParamData
again, which shouldn't happen, because rc
gets set to rc1
, which is SQL_ERROR
, not SQL_NEED_DATA
.For procedural ODBC, the fix would probably be this, but I haven't tested it, just built:
diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c
index a26a1a43c6..587d312b6a 100644
--- a/ext/odbc/php_odbc.c
+++ b/ext/odbc/php_odbc.c
@@ -1025,7 +1025,11 @@ PHP_FUNCTION(odbc_execute)
rc = SQLParamData(result->stmt, (void*)&fp);
if (rc == SQL_NEED_DATA) {
while ((nbytes = read(fp, &buf, 4096)) > 0) {
- SQLPutData(result->stmt, (void*)&buf, nbytes);
+ SQLRETURN rc1 = SQLPutData(result->stmt, (void*)&buf, nbytes);
+ if (!(rc1 == SQL_SUCCESS || rc1 == SQL_SUCCESS_WITH_INFO)) {
+ rc = rc1;
+ break; /* out of the read loop */
+ }
}
}
}
I would say that makes sense. Strangely, the similar loop in PDO_ODBC does not break upon failure. The PDO_ODBC code strikes me a bit as odd:
Something I did some time ago to reliably test rare error conditions in .phpt is to interpose the function. Something like this:
Perhaps this information is useful for you if you want to write a test for a potential PR for this.
Yeah, I think the loop you linked in PDO_ODBC is basically an adapted version of the one in procedural ODBC and should also be fixed - and both are for bound file streams (though the one in procedural ODBC operating against directly a file descriptor that it opens is slightly wild - I've had users accidentally trigger that before!).
Now knowing what caused the failure, I should be able to recreate it consistently with the same database and driver, at least. I'm hoping i.e. MariaDB's ODBC driver exhibits similar behaviour, so it'll be easier to reconstruct.
Description
Edit: I blamed the wrong extension initially, but the problem remains the same. See the comment I posted for what I actually think is going on now that I understand more here.
A user reported to me an issue where
PDO_ODBCprocedural ODBC was returning a function sequence error. Unfortunately, it's a little hard to reproduce independently (large Laravel application, large dataset), but I can clearly see what's causing the error, and that it's obscuring another error that's the source of the actual problem.The relevant part of the unixODBC trace (I'm not posting the full file because it's ~100k lines):
As you can see, there's some
DATA_AT_EXEC
going on (the unixODBC trace makes it hard to determine what parameter is bound as such),so PDO_ODBC starts with anSQLParamData
, and callsSQLPutData
(alternative call site) in a loop as needed.The problem is that failures in
SQLPutData
are not correctly handled. As you can see in the trace,one of the latertheSQLPutData
call is returning an error (unfortunately, unixODBC tracing doesn't decide to find specifics on its own), butPDO_ODBCprocedural ODBC doesn't actuallydo anything with the error code it stores (if it stores it at all)store the error and act on it. Instead, it continues on, callingSQLParamData
again, which is now in an invalid state and within its rights to returnHY010
(as documented, though it's not clear that only successfulSQLPutData
or exec calls are valid).The actual error we care about is for the
SQLPutData
call, but that's not being surfaced, and the misleading function sequence error is returned instead because it ignores that previous error.If it helps, I also have the application's Laravel log from trying to insert the rows, although this does include the user's data. I'm also trying to get the schema and mapping from them as well. It's using the Db2i driver as well, but on Linux.
PHP Version
PHP 8.1.16
Operating System
Ubuntu 22.04