Views result cache ignores query arguments.

Oh, wow ... Nice catch!

The fix is simple, but the tests may not be. Anyway, tagging. The closely related #2452317: Let views result cache use cache contexts recently was fixed.

Looks like a duplicate of #1055616: Query arguments should be replaced before generating cache ID.

Especially https://www.drupal.org/node/1055616#comment-9513975 is very important.

Hi , My patch to resolved your problem.

Index: core/modules/views/src/Plugin/views/cache/CachePluginBase.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- core/modules/views/src/Plugin/views/cache/CachePluginBase.php	(date 1428925949000)
+++ core/modules/views/src/Plugin/views/cache/CachePluginBase.php	(revision )
@@ -284,16 +284,21 @@
       foreach (array('query', 'count_query') as $index) {
         // If the default query back-end is used generate SQL query strings from
         // the query objects.
+        $query = '';
         if ($build_info[$index] instanceof Select) {
+
+          //Clone Object and preExecute is not perfect !
           $query = clone $build_info[$index];
-          $query->preExecute();
-          $build_info[$index] = (string)$query;
+          //$query->preExecute();
+
+          $key_data['build_info'][$index] = array(
+            'sql' => (string) $query,
+            'arguments' => ($query->getArguments()) ? $query->getArguments() : '',
+          );
         }
+
       }
 
-      $key_data = [
-        'build_info' => $build_info,
-      ];
       // @todo https://www.drupal.org/node/2433591 might solve it to not require
       //    the pager information here.
       $key_data['pager'] = [

As per #17, this needs a test.

My commit solves the problem ? no ?

#19: Yes, it does! Great work!

But we need to ensure that no-one changes this code back, hence we need a "test-only" patch that shows the tests failing and exposes the bug.

+++ b/core/modules/views/src/Plugin/views/cache/CachePluginBase.php
@@ -287,7 +287,10 @@ public function generateResultsKey() {
+            'arguments' => $query->getArguments()

We need a comma at the end of the last array element.

See: https://www.drupal.org/coding-standards#array

Add a comma at the end.

Still needs the test.

And here's the patch, including the test.. I believe the interdiff is irrelevant in this case.

@jcnventura thanks for the test on this. It would be useful to provide a test only patch for this.

Uploading a test-only patch as requested.

RTBC +1

I have read all of the lines of the patch and would recommend that this can be accepted as is.

+++ b/core/modules/views/src/Tests/Plugin/CacheTest.php
@@ -101,6 +102,84 @@ public function testTimeResultCaching() {
+        'output_lifespan' => '3600'

Just noticed a small thing.

needs a comma at the end of the line

Added the commas where needed

Create new version of patch removing deprecated drupal_render() function.

I have seen that in some areas of the code short array syntax is used.

We should extend throughout the test?

RTBC

#32: Explicitly checking for the filter plugin does make sense here IMHO as we rely on it later and that is what asserts are for.

RTBC +1

So fix is the same as 7.x, and the test coverage looks exactly what we need.

Committed/pushed to 8.0.x, thanks!