Each QueryPlaceholderInterface implementation are re-computing a unique id on clone events. SelectQuery is not.
Current __clone code is:

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']);
    }
  }

It lacks the parent::__clone(); or $this->uniqueIdentifier = uniqid('', TRUE);.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

regilero’s picture

Status: Active » Needs review
FileSize
508 bytes

Here is the simple patch (very very simple).

Aron Novak’s picture

Status: Needs review » Reviewed & tested by the community

I faced with an issue in Views. That module tries to compute a cache ID from the content of the query, it clones the query but the cloning is far from perfect, somehow the args were modified by __toString, even called on clone, resulting in broken views. After applying this patch, the issue disappeared. The patch seems to be RTBC for me then, it solves a real problem.

David_Rothstein’s picture

Version: 7.x-dev » 8.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs backport to D7

This looks like it needs to go in Drupal 8 first.

Aron Novak’s picture

Status: Needs work » Needs review
FileSize
543 bytes

Rerolled to 8.x

regilero’s picture

Issue summary: View changes

Common guys. This is a real core bug in D7, and the patch does fix it. And then One year later we are waiting for it to be applied on D8 which is a completly different code base?

dawehner’s picture

Issue tags: +Needs tests

Well, we have a backport policy, see https://www.drupal.org/node/767608
Changing this behaviour is not part for this issue, though you can have "fun" starting that discussion again.

Can we at least have tests to proove that the fix actually really works?

pounard’s picture

Backport policy is good for features, but really not for bugfixes that cause real life problems on production sites, the policy should be the other way arround for bugs.

I'll try to write the test when I'll have some spare time.

Damien Tournoud’s picture

There are some other obvious things missing in there:

  • The $this->prepared flag should be nuked;
  • The tables in $this->tables that are implementations of SelectInterface need to be cloned too

But more importantly, this cannot really work because alter hooks are allowed to modify the query object ad-lib. That makes preparation non reentrant, so we should either:

  • Clone the full query object before preparing it in ->execute() (probably with a non-negligible performance impact);
  • Only allow queries to be prepared and executed once, and forbid queries from being cloned if they are prepared

This whole concept of "compiled" vs "prepared" for Select queries needs to be rethought.

Damien Tournoud’s picture

Status: Needs review » Needs work

Anyway, needs work for tests.

pounard’s picture

Your comments make sense, and we can easily forbid objects to be cloned once prepared, which eradicates the need of nuking $this->prepared. We don't really need to rethink the prepared concept as long as we forbid cloning when prepared.

Nice catch for the tables objects, and it's also true for many other objects such as conditions (and maybe others) which can be subqueries.

Damien Tournoud’s picture

Nice catch for the tables objects, and it's also true for many other objects such as conditions (and maybe others) which can be subqueries.

Those should already be cloned by the condition object they belong to.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
quietone’s picture

Status: Needs work » Closed (duplicate)
Issue tags: -Needs backport to D7, -Needs tests +Bug Smash Initiative
Related issues: +#2142107: Complex cloned query dependent on __toString() call

A little digging and found that this was fixed in 8.x in #2142107: Complex cloned query dependent on __toString() call

The D7 issue is #3000191: Complex cloned query dependent on __toString() call

Therefore, closing as a duplicate and moving credit there. If that is wrong, reopen the issue, by setting the status to 'Active', and add a comment.

Thanks!