Conversation
kamil-tekiela
force-pushed
the
Fix-stmt_store_result-error
branch
from
4951305 to
e168109
Compare
|
Unless the bugfix would break reasonably written code or is considered somewhat unsafe/experimental, the fix should target PHP-7.4 or maybe 8.0. |
kamil-tekiela
force-pushed
the
Fix-stmt_store_result-error
branch
from
e168109 to
086f16b
Compare
|
@cmb69 I did |
| /* Nothing to store for UPSERT/LOAD DATA*/ | ||
| if (!mysqlnd_stmt_check_state(stmt)) { | ||
| SET_CLIENT_ERROR(conn->error_info, CR_COMMANDS_OUT_OF_SYNC, UNKNOWN_SQLSTATE, mysqlnd_out_of_sync); | ||
| SET_CLIENT_ERROR(stmt->error_info, CR_COMMANDS_OUT_OF_SYNC, UNKNOWN_SQLSTATE, mysqlnd_out_of_sync); |
There was a problem hiding this comment.
Something that's not really clear to me is when we should be setting the error on the stmt only, and when we should be setting it on stmt and conn both. It seems like in this code we often set both, but it's not really consistent.
There was a problem hiding this comment.
These errors are an absolute mess. It should be that when a function starts with mysql_stmt_ we set stmt->error_info only. For everything else we set conn->error_info. There is a problem with out of memory errors. There are also some exceptions to this rule e.g. prepare method which needs to set both errors as it can be called in both ways.
|
@kamil-tekiela, yes, that is basically what I meant. But for some reason AppVeyor did still pick the master dependencies. I'm not quite sure regarding the rebase command; is "master" the remote? |
|
Merged as c93b461. |
3 participants
Fixes https://bugs.php.net/bug.php?id=80837