ID: 40417 User updated by: exaton at free dot fr Reported By: exaton at free dot fr Status: Bogus Bug Type: PDO related Operating System: Windows XP Pro SP2 PHP Version: 5.2.1 New Comment:
Wow. I'm flabbergasted. Mr Alshanetsky, I am, as they say, and until further notice, disappointed in you. No update of the CVS code. Not even a note in the manual to reflect this spec change. I guess this is going to have to wait until someone else reports it. It's got to be relatively common, especially in not too complex search engine implementations. Until then, therefore... Previous Comments: ------------------------------------------------------------------------ [2007-02-24 03:19:49] [EMAIL PROTECTED] Thank you for taking the time to write to us, but this is not a bug. Please double-check the documentation available at http://www.php.net/manual/ and the instructions on how to report a bug at http://bugs.php.net/how-to-report.php . ------------------------------------------------------------------------ [2007-02-16 11:26:30] exaton at free dot fr Hey again people, I don't mean to be annoying, but I've just done a bit more research, so I thought I'd share it with you. Iliaa, I found the code change where you added the infamous spec-altering error check that I'm going on about : PHP_5_2 : http://cvs.php.net/viewcvs.cgi/php-src/ext/pdo/pdo_sql_parser.c?r1=1.35.2.6.2.3&r2=1.35.2.6.2.4 Also applied to MAIN, with both times the comment : "Added missing check for mismatching number of tokens & bound params in prepared statement emulation." That perfectly matches my error conditions. The problem is, the bindno variable contains the number of individual tokens. However, multiple tokens may have the same name ; but each token name can only be bound once ! So comparing bindno to the number of bindings is incorrect. It introduces the following specification : "multiple tokens may not have the same name in a prepared statement". The workaround is still the same : binding enough bogus tokens to match the number of individual tokens used in the prepared statement, when some share the same name. Oh, did I mention that this prevented anyone with prepared statements containing multiple tokens sharing the same name from upgrading to PHP 5.2.1 ? :-) ------------------------------------------------------------------------ [2007-02-10 17:18:20] exaton at free dot fr OK, I've taken a look at the source code to try and lend a hand in clearing up this issue. My first time though, so here's hoping I'm not too far off the mark. Diffing ext/pdo/ and ext/pdo_pgsql/ files between PHP 5.2.0 and 5.2.1, I find that the error message I am encountering is due to a new paragraph having been *added* to the much remangled ext/pdo/pdo_sql_parser.c (line 262) : if (params && bindno != zend_hash_num_elements(params) && stmt->supports_placeholders == PDO_PLACEHOLDER_NONE) { pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "number of bound variables does not match number of tokens" TSRMLS_CC); ret = -1; goto clean_up; } Somehow I'm trigerring the error condition, here. I'm guessing that my bindno is different from the number of elements in the params hash table. bindno is incremented on line 214. I could be wrong, but I'm under the impression that it is *incremented with each _placeholder_*, which in turn I take to be the "token *instances*" we were talking about before. Now, I think we both agree that we only have to bind as many values/vars as there are *different* tokens in the statement. That is in any case how things worked up to PHP 5.2.0. With the new error detection that has been added (the above paragraph of code), and if I'm right about the way bindno is counted, then we are expected to bind as many values/vars as there are *placeholders* in the statement, even if there are 2 or more placeholders for the same token name. That would be very coherent with the new error I am getting. It would also be coherent with my workaround, in which one just had to bind extra, bogus values/vars (thus artificially filling up the params hash table, with params = stmt -> bound_params) in order to not get this error. So : 1) The new error detection breaks existing scripts that worked with 5.2.0. 2) I think we agree that the specification introduced by this new error detection is incorrect. One may, as far as I know, use several times the same placeholder for bound values/vars in a statement. It is only possible to bind a given token once (because that binding fills a hash table, which will of course not increase in size if the same token is bound several times). Therefore, forcing one to bind as many values/vars as there are *placeholders* is surely incorrect. 3) The workaround is symptomatic of something real fishy going on (having to write bogus code to "unblock" a piece of functionality, wt... ?). That's as much as I can do guys, I have no setup whatsoever for tracing variables in the code. The object of such a trace would be to confirm that, with my test case (in which there are 2 identical ":id" placeholders in the statement), bindno = 2 versus only 1 entry in the params = stmt -> bound_params hash table. Good luck, and thank you for your patience, I'm not much good at writing simple sentences :) ------------------------------------------------------------------------ [2007-02-10 16:18:08] exaton at free dot fr I'm sorry, but I don't understand your reply. Perhaps my use of the word "token" is wrong. The multiple instances "count[ing] as one token, since they reference the same bound parameter" is exactly the behaviour I expect, and had been counting on *and getting* up to now. However, Re. my initial example and test case, *several bindings* are required, *as many as there are _instances_*, not just tokens. This is what is happening in PHP 5.2.1. 1) It differs from what I believe we both understand to be the correct behaviour. 2) Even if "binding as many times as there are *instances* of tokens" were the correct behaviour, we would have to conclude that the implementation was flawed up to PHP 5.2.0, because *existing code* breaks with the upgrade to PHP 5.2.1. My apologies again for insisting, but I think we're both on the same track here, with PHP 5.2.1 having introduced an incoherence on this point (maybe the code in pdo_pgsql.c compares the number of effective bindings to the number of ":<token>" instances instead of the number of effectively different tokens...). ------------------------------------------------------------------------ [2007-02-10 16:07:29] [EMAIL PROTECTED] Thank you for taking the time to write to us, but this is not a bug. Please double-check the documentation available at http://www.php.net/manual/ and the instructions on how to report a bug at http://bugs.php.net/how-to-report.php When you have multiple instances of :id they count as one token since they reference the same bound parameter. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/40417 -- Edit this bug report at http://bugs.php.net/?id=40417&edit=1