Views failing to cache effectively when offsets are used in timestamp filters (D6 -> D7 regression) [#1469188]

When someone filters on a timestamp field (ie a date field) and uses the offset to say something like 10 days ago (ie -10 day) in the filter's settings this breaks the time based caching.

The reason being that DBTNG's preExecute() function, in plugins/views_plugins_cache.inc, is converting ***CURRENT_TIME*** to it's timestamp value before the cache key is determined. Hence in lieu of the the ***CURRENT_TIME*** placeholder appearing in the build_info array the timestamp appears. The end result is the cache key is different every second and therefore the cached is only valid for 1 second. :(

timestamp in the SQL string used to compute the cache key in D7/Views3

node_revision.timestamp >= 1330976131-864000

timestamp in the SQL string used to compute the cache key in D6

node_revision.timestamp >= ***CURRENT_TIME*** - 864000

the code at issue here is line 264 for the plugins/views_plugins_cache.inc

259       foreach (array('query','count_query') as $index) {
260         // If the default query back-end is used generate SQL query strings from
261         // the query objects.
262         if ($build_info[$index] instanceof SelectQueryInterface) {
263           $query = clone $build_info[$index];
264           $query->preExecute();
265           $build_info[$index] = (string)$query;
266         }
267       }

I don't know the fix right off the top of my head, though I know for the time being we're gonna overwrite the handler with our own get_results_key(0 and get_output_key. Seems to me the fix is one of two things.

1.) Get ***CURRENT_TIME*** passed as a placeholder ala :db_condition_placeholder_x in build_info['query'] (and build_info['count_query']), which would have to happen in the views_handlers_filter_date handler
2.) Find a way to get the meta data form the SelectQuery objects

2 will be much more ugly than 1. :)

Comments

Comment #1

effulgentsia CreditAttribution: effulgentsia commented 6 March 2012 at 00:58

Title:

offset timestamp breaks time based cache handler

» Views failing to cache effectively when offsets are used in timestamp filters (D6 -> D7 regression)

Comment #2

dawehner

German

CreditAttribution: dawehner commented 6 March 2012 at 08:11

Does #1469950: "Query results" caching does not respect changed values of exposed filters work for you?

Comment #3

bigjim CreditAttribution: bigjim commented 6 March 2012 at 15:52

No it's still the same issue

Comment #4

bigjim CreditAttribution: bigjim commented 6 March 2012 at 17:34

The easiest solution proposed to date is to use the Date filter which formats the SQL as:

(DATE_FORMAT(ADDTIME(FROM_UNIXTIME(node_revision.timestamp), SEC_TO_TIME(-18000))

I'm assuming we can change the data handler to follow this pattern, but hasn't looked yet.

Comment #5

netw3rker CreditAttribution: netw3rker commented 6 March 2012 at 20:16

One of the problems with changing the date filter to the proposed sql is that there are performance issues with running functions against fields in the where clause. this forces a table scan because the logic has to be applied to every row in the table. The better bet is going to be to use the mysql specific function: "unix_timestamp()" rather than php's request date.

luckily the node_revision.timestamp field is still using ***current_time*** see:

  /**** views/handlers/views_handler_filter_date.inc  @ line 141***/

  function op_simple($field) {
    $value = intval(strtotime($this->value['value'], 0));
    if (!empty($this->value['type']) && $this->value['type'] == 'offset') {
      $value = '***CURRENT_TIME***' . sprintf('%+d', $value); // keep sign
    }
    // This is safe because we are manually scrubbing the value.
    // It is necessary to do it this way because $value is a formula when using an offset.
    $this->query->add_where_expression($this->options['group'], "$field $this->operator $value");
  }

the problem is that the cache key is being generated after the tokens in the sql are replaced:

/* views/plugins/views_plugin_cache.inc @ line 259 */

        if ($build_info[$index] instanceof SelectQueryInterface) {
          $query = clone $build_info[$index];
          $query->preExecute();
          $build_info[$index] = (string)$query;
        }

this has to be done this way because modules like organic groups use tokens like "***current_group***". if the query was cached before tokenization then every group would see the same information, but the revision time would work.. so its a catch 22.

you could probably get away with changing the value that the token ***current_time*** uses by declaring a better value (such as unix_timestamp() ) in a custom module that implements hook_views_query_substitutions()

Hope this helps!

Comment #6

bigjim CreditAttribution: bigjim commented 9 March 2012 at 14:58

We couldn't get hook_views_query_substituions() working as it seems on only want you to add placeholders not replace current ones.

The following fixes this issue:

/**
 * Implements hook_views_query_alter().
 */
function <module_name>_views_query_alter(&$view, &$query) {
  // Replace the ***CURRENT_TIME*** placeholder with unix_timestamp()
  // Note: This function is MYSQL compliant only, it will fail in Postgres, SQLlite, Oracle, SQL
  // Server etc...
  // see http://drupal.org/node/1469188 to learn more about the reasoning
  foreach ($query->where as $where_count => $where) {
    if (count($where['conditions'])) {
      foreach($where['conditions'] as $key => $condition) {
        if ( is_array($condition) && isset($condition['field']) && is_string($condition['field']) && substr_count($condition['field'], '***CURRENT_TIME**')) {
          $query->where[$where_count]['conditions'][$key]['field'] = strtr($condition['field'],array('***CURRENT_TIME***' => 'unix_timestamp()'));
        }
      }
    }
  }
}