php / pecl-database-ibm_db2

Extension for IBM DB2 Universal Database, IBM Cloudscape, and Apache Derby
http://pecl.php.net/package/ibm_db2
Apache License 2.0
23 stars 27 forks source link

Take references instead of symbol names for bind param #16

Open NattyNarwhal opened 3 years ago

NattyNarwhal commented 3 years ago

(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.

NattyNarwhal commented 3 years ago

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?

kadler commented 3 years ago

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.

tessus commented 3 years ago

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. Unfortunately I can't recall the specifics. It has been a while...

praveen-db2 commented 2 years ago

@NattyNarwhal can we close this ?

NattyNarwhal commented 2 years ago

No, this still should be done (unless it's determined we can leave it alone forever). It'd just be a lot of work.