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.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 :)

------------------------------------------------------------------------

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

Reply via email to