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

sbrattla’s picture

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

pounard’s picture

Agree this is a bad issue too, needs fixing.

Version: 7.14 » 7.x-dev

Core issues are now filed against the dev versions where changes will be made. Document the specific release you are using in your issue comment. More information about choosing a version.