I couldn't find an issue reporting this bug, so here it is :
We notice that we are unable to cache a view in Commerce Kickstart without loosing some informations. (see #1691142: Make it possible to cache collection views).

This view is in charge of displaying nodes attached to the term passed as parameter and group them according to terms from another vocabulary. When cache is enabled we loose grouping informations.

Query without cache

SELECT DISTINCT taxonomy_term_data_node.tid AS taxonomy_term_data_node_tid, taxonomy_term_data_node.name AS taxonomy_term_data_node_name, taxonomy_term_data_node.vid AS taxonomy_term_data_node_vid, taxonomy_term_data_node__taxonomy_vocabulary.machine_name AS taxonomy_term_data_node__taxonomy_vocabulary_machine_name, node.nid AS nid
FROM
{node} node
LEFT JOIN (SELECT td.*, tn.nid AS nid
FROM
{taxonomy_term_data} td
LEFT JOIN {taxonomy_vocabulary} tv ON td.vid = tv.vid
LEFT JOIN {taxonomy_index} tn ON tn.tid = td.tid
WHERE (tv.machine_name IN (:db_placeholder_0)) ) taxonomy_term_data_node ON node.nid = taxonomy_term_data_node.nid
LEFT JOIN {taxonomy_vocabulary} taxonomy_term_data_node__taxonomy_vocabulary ON taxonomy_term_data_node.vid = taxonomy_term_data_node__taxonomy_vocabulary.vid
WHERE (( (node.status = 1 OR (node.uid = 1 AND 1 <> 0 AND 1 = 1) OR 1 = 1) AND (node.nid IN (SELECT tn.nid AS nid
FROM
{taxonomy_index} tn
WHERE ( (tn.tid = :db_placeholder_1) ))) ))
ORDER BY taxonomy_term_data_node_name ASC

Query with cache

SELECT DISTINCT taxonomy_term_data_node.tid AS taxonomy_term_data_node_tid, taxonomy_term_data_node.name AS taxonomy_term_data_node_name, taxonomy_term_data_node.vid AS taxonomy_term_data_node_vid, taxonomy_term_data_node__taxonomy_vocabulary.machine_name AS taxonomy_term_data_node__taxonomy_vocabulary_machine_name, node.nid AS nid
FROM
{node} node
LEFT JOIN (SELECT td.*, tn.nid AS nid
FROM
{taxonomy_term_data} td
LEFT JOIN {taxonomy_vocabulary} tv ON td.vid = tv.vid
LEFT JOIN {taxonomy_index} tn ON tn.tid = td.tid
WHERE (tv.machine_name IN (:db_placeholder_0_0)) ) taxonomy_term_data_node ON node.nid = taxonomy_term_data_node.nid
LEFT JOIN {taxonomy_vocabulary} taxonomy_term_data_node__taxonomy_vocabulary ON taxonomy_term_data_node.vid = taxonomy_term_data_node__taxonomy_vocabulary.vid
WHERE (( (node.status = 1 OR (node.uid = 1 AND 1 <> 0 AND 1 = 1) OR 1 = 1) AND (node.nid IN (SELECT tn.nid AS nid
FROM
{taxonomy_index} tn
WHERE ( (tn.tid =:db_placeholder_0_0) ))) ))
ORDER BY taxonomy_term_data_node_name ASC

After digging into views cache system I found that it's probably the fault of views_plugin_cache::get_results_key() function which enforce the view query to be compiled. Then when the query is about to be executed it is recompiled but as part of it has already been compiled, some placeholder goes wrong. And so part of the query failed, that's why we are missing some informations.

Here is the faulty line
views_plugin_cache.inc line 270 :
$build_info[$index] = (string) $query;

Files: 
CommentFileSizeAuthor
#21 1792380-Condition-clone-value-d7-20-test.patch1.64 KBtheo_
FAILED: [[SimpleTest]]: [MySQL] 39,690 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#21 1792380-Condition-clone-value-d7-20.patch2.5 KBtheo_
PASSED: [[SimpleTest]]: [MySQL] 39,732 pass(es).
[ View ]
#18 1792380-Condition-clone-value-18.patch2.55 KBtheo_
PASSED: [[SimpleTest]]: [MySQL] 49,635 pass(es).
[ View ]
#15 1792380-Condition-clone-value-test-15.patch1.68 KBtheo_
FAILED: [[SimpleTest]]: [MySQL] 42,804 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#15 1792380-Condition-clone-value-15.patch2.6 KBtheo_
PASSED: [[SimpleTest]]: [MySQL] 49,412 pass(es).
[ View ]
#14 1792380-Condition-clone-value-test-14.patch1.42 KBtheo_
FAILED: [[SimpleTest]]: [MySQL] 42,250 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#14 1792380-Condition-clone-value-14.patch2.33 KBtheo_
PASSED: [[SimpleTest]]: [MySQL] 42,258 pass(es).
[ View ]
#13 1792380-Condition-clone-value-test.patch1.66 KBtheo_
FAILED: [[SimpleTest]]: [MySQL] 42,082 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#13 1792380-Condition-clone-value-13.patch2.58 KBtheo_
PASSED: [[SimpleTest]]: [MySQL] 42,071 pass(es).
[ View ]
#6 1792380-Condition-clone-value.patch967 bytestheo_
PASSED: [[SimpleTest]]: [MySQL] 41,896 pass(es).
[ View ]
#2 1792380-DatabaseCondition-clone.patch909 bytestheo_
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1792380-DatabaseCondition-clone.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Good spot of the actual problem!

Mh we had this issue before before views got released but i was sure this once worked.
There have been even a core issue: #1112854: Subqueries use wrong arguments.

I tried to reproduce the problem using some custom code but failed, maybe it is helpful for you.

$subquery = db_select('users')
->fields('users', array('uid'));
$or = db_or()
->condition('status', 1)
->condition('status', 0);
$subquery->condition($or);
dsm((string) $subquery);
$select = db_select('node')
->condition('type', 'story')
->condition('uid', $subquery, 'IN');
$clone = clone $select;
dsm((string) $clone);
$clone->preExecute();
dsm((string) $select);

Project:Views» Drupal core
Version:7.x-3.5» 7.x-dev
Component:Code» database system
Status:Active» Needs review
StatusFileSize
new909 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1792380-DatabaseCondition-clone.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Finally i found that it's a core issue related to the __clone method of the DatabaseCondition class.
When you clone a DatabaseCondition object containing a value which is a SelectQuery, the value object is not cloned and it should be.

For example views cache system does the following :
clone the query
compile the cloned query (the value object is compiled on the original query too)
execute the original query -> fail cause we are unable to re-compile query cause of value which is already compiled

So here is a patch to take care about cloning the value object.

Title:Views plugin cache issueDatabaseCondition not cloning SelectQuery value object

Status:Needs review» Needs work

The last submitted patch, 1792380-DatabaseCondition-clone.patch, failed testing.

Version:7.x-dev» 8.x-dev

Thanks for working on this issue!

Things like that have to be fixed first in drupal8 and then get backported

Status:Needs work» Needs review
StatusFileSize
new967 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,896 pass(es).
[ View ]

Here is the patch for drupal8

Issue tags:+Needs tests

There should be for sure a test which proves that the current behavior is actually broken.

Here is the way to reproduce it in both d7 & d8.

$user = db_select('users')
  ->fields('users', array('uid'));
$or = db_or()
  ->condition('status', 1)
  ->condition('status', 0);
$user->condition($or);
$type = db_select('node_type')
  ->fields('node_type', array('type'));
$or = db_or()
  ->condition('module', 'node')
  ->condition('locked', 0);
$type->condition($or);
$select = db_select('node');
$select->condition('uid', $user, 'IN');
$clone = clone $select;
$clone->preExecute();
dsm((string) $clone);
$select->condition('type', $type, 'IN');
dsm((string) $select);

#2: 1792380-DatabaseCondition-clone.patch queued for re-testing.

anyone ?

What about document that this might be a subquery? In general i would say this looks fine but i'm not an expert in the database system.

It does look fine; we need a standard for clone, I really dislike clone() it's wrong though it works , clone is not a function; it needs tests.

StatusFileSize
new2.58 KB
PASSED: [[SimpleTest]]: [MySQL] 42,071 pass(es).
[ View ]
new1.66 KB
FAILED: [[SimpleTest]]: [MySQL] 42,082 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Here is first the failing test, then the test + patch

StatusFileSize
new2.33 KB
PASSED: [[SimpleTest]]: [MySQL] 42,258 pass(es).
[ View ]
new1.42 KB
FAILED: [[SimpleTest]]: [MySQL] 42,250 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

I found that previous tests are not relevant since this issue #1808624: SelectQuery __clone method does not recompute the Query uniqueIdentifier while other QueryPlaceholderInterface does fix it.

So here is a new test which expose the issue in a more obvious way.

StatusFileSize
new2.6 KB
PASSED: [[SimpleTest]]: [MySQL] 49,412 pass(es).
[ View ]
new1.68 KB
FAILED: [[SimpleTest]]: [MySQL] 42,804 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Improve test using countQuery

Issue tags:-Needs tests

The fix and especially the tests are looking great so far, so remove the tags.
Maybe damien is a good person to finally review this change.

+++ b/core/modules/system/lib/Drupal/system/Tests/Database/SelectCloneTest.phpundefined
@@ -0,0 +1,47 @@
+ * Definition of Drupal\system\Tests\Database\SelectCloneTest.

Just a small nitpick: The current standard in http://drupal.org/node/1354 says "Contains \Drupal...."

StatusFileSize
new2.55 KB
PASSED: [[SimpleTest]]: [MySQL] 49,635 pass(es).
[ View ]

Using correct standard

Status:Needs review» Reviewed & tested by the community

I think this seems reasonable.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)
Issue tags:+needs backport to D7

Committed/pushed to 8.x, thanks!

Moving to 7.x for backport.

Status:Patch (to be ported)» Needs review
StatusFileSize
new2.5 KB
PASSED: [[SimpleTest]]: [MySQL] 39,732 pass(es).
[ View ]
new1.64 KB
FAILED: [[SimpleTest]]: [MySQL] 39,690 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Backported to D7

Status:Needs review» Reviewed & tested by the community

Title:DatabaseCondition not cloning SelectQuery value objectDatabaseCondition not cloning SelectQuery value object (documentation followup)
Version:7.x-dev» 8.x-dev
Status:Reviewed & tested by the community» Active

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/ad244e2

Pretty sure this makes the PHPDoc on that __clone() method out of date, though?

Yes, you are right.

We might have something like :
"Copy conditional statements and subqueries." instead of the actual "Only copies fields that implement Drupal\Core\Database\Query\ConditionInterface."