Updated: Comment #10

Problem/Motivation

Too many caches are generated in table cache_views_data in the following case:
- a Views view with more then one display,
- and caching enabled,
- and each display/several displays having the same result, but a different output (e.g. chart and table)

For a complicated view, the results cache and the output cache save notable time.
If a user switches between displays, there is no need to fetch the database data again, if it is already stored in the results cache.

Proposed resolution

Below is a patch that eliminates querying the database a second time, when switching between displays of the same query.
#1055616: Query arguments should be replaced before generating cache ID is a prerequisite, since that issue makes sure the cache key is built correctly.
The display name is discarded from the output key name. This might be a problem in some cases, but IMO only serves documentation purposes.

Comments

StatusFileSize
new1.81 KB
FAILED: [[SimpleTest]]: [MySQL] 1,448 pass(es), 1 fail(s), and 8 exception(s).
[ View ]

Please find a patch attached.
I am not sure what is better: explicitly add elements from $build_info, (like above),
or explicitly unset($key_data['buildinfo']['title']) and unset($key_data['buildinfo']['breadcrumb'])

Status:Needs review» Needs work

The last submitted patch, views_1606292_1_cache_output_key.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.73 KB
FAILED: [[SimpleTest]]: [MySQL] 1,448 pass(es), 1 fail(s), and 2 exception(s).
[ View ]

It seems better to unset() data...

Status:Needs review» Needs work

The last submitted patch, views_1606292_3_cache_result_key.patch, failed testing.

StatusFileSize
new1.17 KB
PASSED: [[SimpleTest]]: [MySQL] 1,451 pass(es).
[ View ]

I am not sure why the test fails.
The new code from May-18 from #1530740-4: views cache adds all css/js on hits unnecessarily is the cause.

Attached patch is almost identical to #1055616-30: Query arguments should be replaced before generating cache ID, which only considers the hash-data.
[EDIT: disregard this patch. It is in error.]

Only the last line is extra, which removes the $display->id from the results_key.
I am not sure what depends on that piece of key, I suppose it is only to have some human_readable info, and to invalidate cache of a specific display.

Status:Needs work» Needs review

resetting status for testbot.

Status:Needs review» Postponed

Things are changing in #1055616: Query arguments should be replaced before generating cache ID enough that this can just be a followup.

Agreed,
then this will be the only change for this request:

<?php
--- plugins/views_plugin_cache.inc    (revision 1620)
+++
plugins/views_plugin_cache.inc    (working copy)
@@ -
261,7 +261,8 @@
   function
get_results_key() {
     if (!isset(
$this->_results_key)) {
-     
$this->_results_key = $this->view->name . ':' . $this->display->id . ':results:' . $this->get_cache_key();
+     
$this->_results_key = $this->view->name . ':' . ':results:' . $this->get_cache_key();
     }
     return
$this->_results_key;
?>

ITMT, perhaps we can check if the presence of $this->display->id is obligatory.

Issue tags:+Needs tests

Yeah this will need tests.

I found one occasion where this change is not compatible:
Views Data Export generates #1822916: Error 'Base table or view not found:' when VDE-display is batched and cached.

[EDIT] But Views Data Export is not compatible with any cache, and ATM all caching form that module is now programmatically disabled.

StatusFileSize
new555 bytes

This is patch #8 as a file. Unfortunately in svn format.