#40417 [Opn]: Suddenly binding as many vars as there are *identical* prepared tokens
ID: 40417
User updated by: exaton at free dot fr
Reported By: exaton at free dot fr
Status: Open
Bug Type: PDO related
Operating System: Windows XP Pro SP2
PHP Version: 5.2.1
New Comment:
*Damn* *it*.
Good on you for spotting that, mgagne. For some freaking reason I was
so bent on examining the documentation for binding methods that I
skipped prepare(). No idea why.
Ilia, my apologies again, therefore ; but really I had not understood
your intermediate reply. At least now we have the knowledge that you
were working with all along.
So now, of course, the question is : should the spec' be changed in
favor of PHP's behaviour up until now ? I of course advocate changing
the manual, allowing for the new feature of multiple parameter
markers with the same name. I argue with the following points :
1) This issue already seems quite popular ; obviously, quite a few
people relied on the feature without realizing that it went against the
true specification.
Now that, of course, is a rather specious argument ; developers working
with PHP should stick to the spec' and get bent if they make use of
unintended functionality which is suddenly dropped -- I'm the first to
agree. However :
2) I see no compelling reason for which PHP should not support this
feature. It is a very natural feature that makes perfect sense when it
is used. It is most practical is naive search engine implementations
for small sites (auto-generated : WHERE (title LIKE :term0 OR text LIKE
:term0 OR author_name LIKE :term0) AND (title LIKE :term1 OR text LIKE
:term1 OR author_name LIME :term1) ...).
The underlying code is already designed in such a way as to support the
feature of multiple parameter markers with the same name. The only
change in PHP 5.2.1 is a condition check that throws an error, based on
the specification. And admittedly, writing that check in conformance
with the feature would be expensive (see e.g. my suggestion above)
and/or cumbersome or complicated, etc.
This would be a call for the PHP core developers.
Previous Comments:
[2007-03-02 21:59:15] mgagne at generationphp dot net
Unfortunately for some people, Iliaa is right:
You cannot use a named parameter marker of the same name twice in a
prepared statement.
However, even if it's was added to the documentation about a year ago
(Sun Jan 8 14:02:42 2006 UTC), the behavior changed between PHP 5.2.0
and PHP 5.2.1 and it should have been added to the changelog as well.
I didn't know when happened to my application until I saw this bug
report.
[2007-03-02 07:25:23] mgagne at generationphp dot net
Hi,
I have the same bug using PHP 5.2.1. I had to downgrade to PHP 5.2.0
and it fixed the problem.
I'm using PDO::MYSQL. I have 2 bound variables in my request. All 2
have the same name. Since I'm only binding value once using
PDO::bindValue, the error is triggered without valid reason.
My query is similar to this one:
SELECT *
FROM posts
WHERE post_title LIKE :q OR
post_text LIKE :q
I'm binding value once like this:
$sth-bindValue(':q', $q, PDO::PARAM_STR);
This means there is something wrong within the internal count.
Also for the records, issue does not seem to be related to any specific
PDO driver. (issue is present with PostGreSQL and MySQL driver)
Thanks
[2007-03-01 15:30:09] exaton at free dot fr
@ xing : I had not seen that word from Wez, but indeed is does make
sense to add the check in principle -- the API should make sure that
enough tokens were bound (to enhance its service and avoid silent
bugs) and can also guarantee that not too many were bound (might as
well).
It's back to the problem with bindno, however... I'm not even sure of
the name of that variable. I think it would stand for number of
bindings, e.g. the expected number of bound variables or values. I
believe that's how Ilia read it, and quite reasonably so. That meaning
is just not valid in the special (but probably not uncommon) case of
multiple named tokens (as opposed to question marks ?) with the same
name.
Just a shot in the dark : wouldn't a workable, albeit expensive,
solution be to create a little hash table here with the names of all
the named tokens ? It would not hold duplicates, by definition ; hence
named tokens with the same name would only be counted once. Therefore
zend_hash_num_elements(params) would just have to be compared to
zend_hash_num_elements(token_names). But of course question-mark
placeholders would have to be treated in a different way...
Anyway, I don't think that's anything the PHP developers won't have
thought about themselves. Just my 2 cents.
[2007-03-01 08:15:50] xing at mac dot com
I have to agree with
#40417 [Opn]: Suddenly binding as many vars as there are *identical* prepared tokens
ID: 40417 User updated by: exaton at free dot fr Reported By: exaton at free dot fr Status: Open Bug Type: PDO related Operating System: Windows XP Pro SP2 PHP Version: 5.2.1 New Comment: @ xing : I had not seen that word from Wez, but indeed is does make sense to add the check in principle -- the API should make sure that enough tokens were bound (to enhance its service and avoid silent bugs) and can also guarantee that not too many were bound (might as well). It's back to the problem with bindno, however... I'm not even sure of the name of that variable. I think it would stand for number of bindings, e.g. the expected number of bound variables or values. I believe that's how Ilia read it, and quite reasonably so. That meaning is just not valid in the special (but probably not uncommon) case of multiple named tokens (as opposed to question marks ?) with the same name. Just a shot in the dark : wouldn't a workable, albeit expensive, solution be to create a little hash table here with the names of all the named tokens ? It would not hold duplicates, by definition ; hence named tokens with the same name would only be counted once. Therefore zend_hash_num_elements(params) would just have to be compared to zend_hash_num_elements(token_names). But of course question-mark placeholders would have to be treated in a different way... Anyway, I don't think that's anything the PHP developers won't have thought about themselves. Just my 2 cents. Previous Comments: [2007-03-01 08:15:50] xing at mac dot com I have to agree with exaton on this. This is an absolute app-breaker change and MUST be noted in the change-log at the very least. It is pure luck I found this change before my official upgrade to 5.2.1. I really hope there there a solution to this. On a blog, wez mentioned that this was a fix and the previous ability to bind one to many placements was rather an bug. I however, strongly disagree on a simple level that the pre-5.2.1 pdo binding just made sense. Why should php force developers to introduce more lines of code that does nothing more when this can be take care of behind the scenes? [2007-02-27 13:00:47] exaton at free dot fr Hi again, thanks for reopening this issue. Sorry for being so snarky before, but I'd just received a little dressing down from my boss because of having to add the workaround to already-validated code at extremely short notice. Classic case of pushing for an upgrade on the production server in the frenzy of the moment. I'll let you guys take care of this now. I've kept my test case around so I'm available for further trials if I can be of any use. [2007-02-27 11:50:41] [EMAIL PROTECTED] This is really annoying issue, which forces me to rewrite some of the code I've done in the past. Perhaps the bindno shouldn't be incremented if the named placeholder already exists in the placeholders struct? Would it break something else? [2007-02-24 08:47:52] exaton at free dot fr 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... [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 . 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=40417edit=1
#40417 [Opn]: Suddenly binding as many vars as there are *identical* prepared tokens
ID: 40417
User updated by: exaton at free dot fr
Reported By: exaton at free dot fr
Status: Open
Bug Type: PDO related
Operating System: Windows XP Pro SP2
PHP Version: 5.2.1
New Comment:
Hi again, thanks for reopening this issue.
Sorry for being so snarky before, but I'd just received a little
dressing down from my boss because of having to add the workaround to
already-validated code at extremely short notice. Classic case of
pushing for an upgrade on the production server in the frenzy of the
moment.
I'll let you guys take care of this now. I've kept my test case around
so I'm available for further trials if I can be of any use.
Previous Comments:
[2007-02-27 11:50:41] [EMAIL PROTECTED]
This is really annoying issue, which forces me to rewrite some
of the code I've done in the past. Perhaps the bindno
shouldn't be incremented if the named placeholder already
exists in the placeholders struct? Would it break something
else?
[2007-02-24 08:47:52] exaton at free dot fr
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...
[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.3r2=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
#40417 [Opn]: Suddenly binding as many vars as there are *identical* prepared tokens
ID: 40417
User updated by: exaton at free dot fr
Reported By: exaton at free dot fr
Status: Open
Bug Type: PDO related
Operating System: Windows XP Pro SP2
PHP Version: 5.2.1
New Comment:
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.3r2=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 ? :-)
Previous Comments:
[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
#40417 [Opn]: Suddenly binding as many vars as there are *identical* prepared tokens
ID: 40417
User updated by: exaton at free dot fr
Reported By: exaton at free dot fr
Status: Open
Bug Type: PDO related
Operating System: Windows XP Pro SP2
PHP Version: 5.2.1
New Comment:
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 :)
Previous Comments:
[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.
[2007-02-09 17:25:31] exaton at free dot fr
All right, here you go, but it's really because I love you guys :)
So, still comparing PHP 5.2.0
