Re: [PHP-DEV] Re: mysqli tests breaking
Am 04.09.2011 06:35, schrieb Stas Malyshev:
The ones I'm most worried about are:
1. mysqli_stmt_num_rows() is expected to return number of rows after all
rows were fetched while returning 0 before that. libmysql definitely
Yes, here the test assumes a marginally different behavior. The test
needs an update. No, there is no BC break.
doesn't do that, in full accordance with mysql docs which state row
numbers can't be had for unbuffered queries.
If we do want this new non-portable semantics in mysqlnd, we should have
it in separate test, clearly marked as such, even though I'm not
convinced having such semantics is a good idea at all (would lead to
applications working with mysqlnd and not with libmysql - invitation for
trouble).
Your description of the difference is wrong. You conclusion invitation
for trouble must be a joke.
a) The difference
Pseudo-code:
/* identical behavior */
mysqli_stmt_prepare(stmt, SELECT ...);
mysqli_stmt_execute(stmt)
do {
assert(0 == mysqli_stmt_num_rows(stmt));
} while (mysqli_stmt_fetch(stmt);
/* difference now */
printf(number of rows fetched: %d\n, mysqli_stmt_num_rows());
Output:
libmysql: 0 rows fetched
mysqlnd: actual_number_of_rows rows fetched
Difference: mysqlnd lifts a silly libmysql limitation.
b) Practical relevance
bugs.php.net - 0 related reports
https://bugs.php.net/search.php?search_for=mysqli_stmt_num_rowsboolean=0limit=Allorder_by=iddirection=DESCcmd=displaystatus=Allbug_type=Allphp_os=phpver=cve_id=assign=author_email=bug_age=0bug_updated=0
code.google.com - nobody using the function ?!
http://code.google.com/intl/en-EN/query/#q=mysqli_stmt_num_rows
Code that relies on undefined behavior:
num_rows = 0;
while (mysql_stmt_fetch(stmt)) {
num_rows++;
}
/* Bogus code - does not follow docs */
if (num_rows 0 !== mysqli_stmt_num_rows(stmt)
printf(Damn, API returns a meaninful value);
else
printf(Jippie, MySQL folks lifted a limitation);
Worst that could happen is that someone relies on stmt_num_rows() @
mysqlnd here. Relying on any specific behavior is bogus. We are talking
about C libmysql (libmysql vs. mysqlnd) differences here, thus we can
check C API reference: [...] Otherwise, the row count is unavailable
unless you count the rows as you fetch them. ,
http://dev.mysql.com/doc/refman/5.6/en/mysql-stmt-num-rows.html .
There is no promise made in the documentation that num_rows is set to 0.
Unavailable means that it can be set to any value. The return value
can be num_rows_last_stmt, 0 (libmysql) or the actual number of rows
(mysqlnd). None of this would violate the documentation.
This leaves us with:
- BC breakage is impossible because behavior is undefined
- update test, issue gone
Ulf
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: mysqli tests breaking
Hi! On 9/5/11 2:29 AM, Ulf Wendel wrote: - BC breakage is impossible because behavior is undefined - update test, issue gone OK, since you say it's not the intended behavior to be relied upon, I'll remove this check from the test (if nobody beats me to it). -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: mysqli tests breaking
On Mon, Sep 5, 2011 at 11:37 AM, Stas Malyshev smalys...@sugarcrm.com wrote: Hi! On 9/5/11 2:29 AM, Ulf Wendel wrote: - BC breakage is impossible because behavior is undefined - update test, issue gone OK, since you say it's not the intended behavior to be relied upon, I'll remove this check from the test (if nobody beats me to it). I will! not! :-) -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: mysqli tests breaking
Am 05.09.2011 11:37, schrieb Stas Malyshev: Hi! On 9/5/11 2:29 AM, Ulf Wendel wrote: - BC breakage is impossible because behavior is undefined - update test, issue gone OK, since you say it's not the intended behavior to be relied upon, I'll remove this check from the test (if nobody beats me to it). No, please don't change it that way. Returning a relevant value for stmt_num_rows() seems a valid feature request that makes perfectly sense to me and is somewhat in line with the vague non-PS documentation of the case. To the end user the message is undefined, do not rely on. Towards the library implementor the message is try to make libmysql become better in the future, keep reasonable value in mysqlnd implementation meanwhile. Do something like: if ($IS_MYSQLND) /* TO END USER: no promise on this assumption */ ... Please, do not drop the information of the difference by removal of the assertion. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: mysqli tests breaking
Hi! On 9/5/11 2:49 AM, Ulf Wendel wrote: Returning a relevant value for stmt_num_rows() seems a valid feature request that makes perfectly sense to me and is somewhat in line with the vague non-PS documentation of the case. It's not a good situation where mysqlnd and libmysql have different semantics and people are encouraged to rely on it (if you call it feature, you encourage people to use it, otherwise why call it feature?). It leads to code that works on mysqnd but not on libmysql, without any indication of it - you'll just install an app, and it would work on one server but not on the other, for unknown reason. But if you add it at least it should be clearly documented - both in the manual and in the tests - that this is a non-portable mysqnd-only semantics. And the tests should be changed accordingly, so libmysql won't fail. To the end user the message is undefined, do not rely on. Towards the library implementor the message is try to make libmysql become better in the future, keep reasonable value in mysqlnd implementation meanwhile. Do something like: if ($IS_MYSQLND) /* TO END USER: no promise on this assumption */ ... Please, do not drop the information of the difference by removal of the assertion. OK, please make the patch yourself, so it looks as you prefer. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: mysqli tests breaking
Am 05.09.2011 12:00, schrieb Stas Malyshev: Hi! On 9/5/11 2:49 AM, Ulf Wendel wrote: Returning a relevant value for stmt_num_rows() seems a valid feature request that makes perfectly sense to me and is somewhat in line with the vague non-PS documentation of the case. It's not a good situation where mysqlnd and libmysql have different semantics and people are encouraged to rely on it (if you call it feature, you encourage people to use it, otherwise why call it feature?). It leads to code that works on mysqnd but not on libmysql, without any indication of it - you'll just install an app, and it would Well, those who want to reply on UNDEFINED behavior, shall fool themselves. It will be a great laugh for the rest of us. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: mysqli tests breaking
Hi! I've tried with latest mysql library and as I suspected, most of the issues still stay. The ones I'm most worried about are: 1. mysqli_stmt_num_rows() is expected to return number of rows after all rows were fetched while returning 0 before that. libmysql definitely doesn't do that, in full accordance with mysql docs which state row numbers can't be had for unbuffered queries. If we do want this new non-portable semantics in mysqlnd, we should have it in separate test, clearly marked as such, even though I'm not convinced having such semantics is a good idea at all (would lead to applications working with mysqlnd and not with libmysql - invitation for trouble). 2. last_insert_id semantics change. The test expects select queries to reset last insert it to 0, which does not happen with libmysql. Maybe it happens on later versions of mysql server, I do not know - but we can't have a test the is bound to specific server version, at least not without a check, and even better - separating it from portable tests. If it's specific to mysqlnd it's even worse - it means that some code that assumes libmysql behavior will be broken, and as last_insert_id() is a very frequently used function I think it's very dangerous. Also, new issue added with recent libmysql - newer builds of libmysql do not declare DBUG_OFF at all in my_config.h. See for example http://bugs.mysql.com/bug.php?id=60872. So we should take the absence of DBUG_OFF as a sign that debug is enabled. I would recommend checking if DEBUG_ON is defined and enabling debug only then. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
