-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Take references instead of symbol names for bind param #16
Comments
I'm also curious as to why ibm_db2 had this weird API decision in the first place. Did references not exist in the language when it was written, and all the other DB extensions changed except it? |
I recall Tony complaining about PHP 7 changes that caused problems for this API and it blew my mind to learn that this API takes a variable name, not a reference to a variable. |
This is a really good question. We were not allowed to look at any other code because of code contamination and legal issues. The only info we had at the time was the rather sparse PHP internals documentation. |
@NattyNarwhal can we close this ? |
No, this still should be done (unless it's determined we can leave it alone forever). It'd just be a lot of work. |
(Writing it in here instead of bugs.php primarily so we can get the feel for GH issues for PHP stuff.)
The fact this API takes a symbol name (which it goes off into the symbol table to frob) gives it really confusing semantics (when exactly is the bind applied? what happens when the scope of bind and exec are different?), especially when classes are involved, and I suspect it's the root cause of many bizarre hard to explain (let alone reproduce) issues we encounter. We've also been special-cased by opcache because of this.
What we should be doing is either change the old API (if it can be done w/o a BC break, or if a BC break is acceptable), or a new API that takes a reference instead. I suspect we need to alter the bind handling a little so it can work with both semantics.
I think this might be preferable since we can't get everyone to use ODBC/PDO_IBM and this would be a smaller change for users than switching to those.
The text was updated successfully, but these errors were encountered: