Providing by reference getter on objects is not a good idea and makes the objects standard behaviors unpredictables.
So, first let's see an example, it's easier to understand the problem wiith an example:
$select = db_select('user','u');
$tables = $select->getTables();
var_dump($select);
//object(SelectQuery)#282 (19) {
// (...)
// ["tables":protected]=> <-- notice, there is no &
// array(1) {
// (...)
// Work on a clone query and connect on node table *****
$clone1 = clone $select;
$clone1->join('node','n','n.uid=u.uid');
// BUGGY line! <- ************************
// adding a reference in operator will transform
// the internal attribute storage from a simple array
// to an by-reference array
$tables =& $select->getTables();
var_dump($select);
// object(SelectQuery)#282 (19) {
// (...)
// ["tables":protected]=> <-- notice, there is a &
// &array(1) {
// (...)
// Work on a new clone query and connect on users_role table *****
$clone2 = clone $select;
$clone2->join('users_roles','r','r.uid=u.uid');
// Work on original query and connect on comment table *****
$select->join('comment','c','c.uid=u.uid');
print_r($select->__toString());
// BUG: users_roles was never added on original query
// SELECT FROM {user} u INNER JOIN {users_roles} r ON r.uid=u.uid INNER JOIN {comment} c ON c.uid=u.uid
print_r($clone1->__toString());
// SELECT FROM {user} u INNER JOIN {node} n ON n.uid=u.uid
print_r($clone2->__toString());
// BUG: comments was never added on second clone query
// SELECT FROM {user} u INNER JOIN {users_roles} r ON r.uid=u.uid INNER JOIN {comment} c ON c.uid=u.uid
The problem here is that I should have 3 independent queries but the original query and the second clone are clearly still dependent and share the same $tables internal attribute. tested on php 5.3.
This is because of "=&" in :
$tables =& $select->getTables();
which refer to the getter of the SelectQuery object:
public function &getTables() {
return $this->tables;
}
If I remove the '&' from that setter and from the SelectQueryInterface the problem disappears and the example gives me 3 independant queries.
SELECT FROM {user} u INNER JOIN {comment} c ON c.uid=u.uid;
SELECT FROM {user} u INNER JOIN {node} n ON n.uid=u.uid;
SELECT FROM {user} u INNER JOIN {users_roles} r ON r.uid=u.uid;
Note that on the example code I never writes anything connecting the tables from the second clone and the original query. The bug is that using a by-reference getter and using it with a by-reference equal operator is altering the way the internal SelectQuery object is storing the $tables internal attributes. In the example the var_dump clearly show that after such usage the internal attributes is now a reference to an array and not a simple array.
Solutions to this problems may be ading a test of referenced attributes in the __clone method which is actually simply:
public function __clone() {
// On cloning, also clone the dependent objects. However, we do not
// want to clone the database connection object as that would duplicate the
// connection itself.
$this->where = clone($this->where);
$this->having = clone($this->having);
foreach ($this->union as $key => $aggregate) {
$this->union[$key]['query'] = clone($aggregate['query']);
}
}
But... PHP as no simple way to detect if an attribute is by reference or not.
The cleaner solution is to forbidd by-reference getter and enforce usage of setters for external objects when they want to save a modified version of the internal objects attributes. It seems quite obvious that breaking the PHP object models with by reference getters is highly unpredictable.
Comments
Comment #1
sbrattla CreditAttribution: sbrattla commentedI highly agree that this is an issue.
The behaviour with "shared" dependencies across different clones is not quite what you would expect. I posted an issue about this at http://drupal.stackexchange.com/questions/41205/clone-a-selectquery-inst....
Comment #2
pounardAgree this is a bad issue too, needs fixing.