Because Views keeps a single array with all table names, no two tables that are being used by a View can have the same name. This is unrealistic when using Views to access multiple databases, and can be a problem for contrib modules which are not aware of each other's naming. The attached patch adds a 'real table' param to table definitions, so you can use an alias to refer to the table in the Views array to avoid collisions. For example, if you need to access a table called 'users' in an external database, you would do this to avoid colliding with the Drupal 'users' table:

$data['other_database_user']['table']['base'] = array(
'real_table' => 'users',
'database' => 'some_other_database',
'field' => 'id',
...
);

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

machinehum’s picture

Mołot’s picture

Status: Needs review » Needs work

+1 to this.
Patch seems to work, but I don't like that approach.

I'd prefer views to use one more column in their index, one specifying where given table came from. Then, in case of collision, default to the same DB that was already used for items in given query, or to the module that tries to use it. Unless, of course, module explicitly specifies it asks for another module's table.
With your approach I must be aware of collision to avoid it. With mine, no human intervention is required unless one wants to.

machinehum’s picture

I agree that indexing on the combination of table name and the 'database' parameter, rather than table name alone, is a better approach. What I was going for here is the smallest change i could make to address the issue, in the hope it'd be more likely to get accepted.

Mołot’s picture

Version: 7.x-3.7 » 7.x-3.x-dev

My point exactly, just from the other side. Your way is a small change for views, but huge burden on module developers. Now if some table will appear in core, it will break modules using same named tables in other databases, and require them to update. Your approach will make it easier to update, but that's pretty much all. My approach removes the need to upgrade to stranger's changes altogether. If I'll get some free time, I'll try to write that... But feel free not to wait for me ;)

And I sure hope others will join discussion here.

PS re-testing your patch against dev, OK?

Mołot’s picture

Status: Needs work » Needs review
Mołot’s picture

OK, passes on dev too. I still don't think it's the best approach, but at least it seems to work and not break anything else.

dobe’s picture

This patch worked perfectly. I have the need to be able to use views to pull data from another drupal installation. Unless I am approaching this wrong. This allows me to do what I need.

-Jesse

dobe’s picture

I am having an issue. Not sure if it is related to this completely. As said in #7 comment, I am trying to pull data from another drupal site. I am able to pull any content type from that other database with its basic fields. What I am having troubles with is grabbing the fields that are attached to the content.

Views builds the query. I can take that query and run in straight on the database and it grabs the fields I need. However, views does not place the data in the view.

Not really sure how to direct my issue.

My hook_views_data() looks like:

<?php
function mymodule_views_data() {
  $data = array();

  db_set_active('engine');
  $views_data = cache_get('views_data:en', 'cache_views');
  db_set_active();

  foreach($views_data->data as $cid => $table){
    $de_cid = 'de_'.$cid;
    if($cid == 'node' || $cid == 'entity_node' || $cid == 'views_entity_node'){
      $data[$de_cid] = $table;
      if(isset($table['table']['group'])){
        $data[$de_cid]['table']['group'] = t('DataEngine: '.$table['table']['group']);
      }
      if(isset($table['table']['base'])){
        $data[$de_cid]['table']['base']['database'] = 'engine';
        $data[$de_cid]['table']['base']['real_table'] = $cid;
        if(isset($table['table']['base']['title'])){
          $data[$de_cid]['table']['base']['title'] = t('DataEngine: '.$table['table']['base']['title']);
        }
      }
      if($cid == 'views_entity_node'){
        foreach($table['table']['join'] as $join => $info){
          $data[$de_cid]['table']['join']['de_'.$join] = $info;
          unset($data[$de_cid]['table']['join'][$join]);
        }
      }
    }
    if(substr_compare($cid, "field_data_", 0, 11) == 0){
      $data[$cid] = $table;
      $field_name = substr($cid, 11);
      if(isset($table['table']['join'])){
        foreach($table['table']['join'] as $join => $info){
          $data[$cid]['table']['join']['de_'.$join] = $info;
          unset($data[$cid]['table']['join'][$join]);
        }
      }
      if(isset($table[$field_name]['field']['entity_tables'])){
        $data[$cid][$field_name]['field']['add fields to query'] = TRUE;
        foreach($data[$cid][$field_name]['field']['entity_tables'] as $entity_table => $value){
          $data[$cid][$field_name]['field']['entity_tables']['de_'.$entity_table] = $value;
          unset($data[$cid][$field_name]['field']['entity_tables'][$entity_table]);
        }
      }
    }
  }
  return $data;
}
?>

The query views produces that works, except in views:

SELECT de_node.title AS de_node_title, de_node.nid AS nid, field_data_field_female_pop_10_to_14_years.delta AS field_data_field_female_pop_10_to_14_years_delta, field_data_field_female_pop_10_to_14_years.language AS field_data_field_female_pop_10_to_14_years_language, field_data_field_female_pop_10_to_14_years.bundle AS field_data_field_female_pop_10_to_14_years_bundle, field_data_field_female_pop_10_to_14_years.field_female_pop_10_to_14_years_value AS field_data_field_female_pop_10_to_14_years_field_female_pop_, field_data_field_female_pop_10_to_14_years.revision_id AS field_data_field_female_pop_10_to_14_years_revision_id, 'node' AS field_data_field_female_pop_10_to_14_years_node_entity_type
FROM 
{node} de_node
LEFT JOIN {field_data_field_female_pop_10_to_14_years} field_data_field_female_pop_10_to_14_years ON de_node.nid = field_data_field_female_pop_10_to_14_years.entity_id AND (field_data_field_female_pop_10_to_14_years.entity_type = 'node' AND field_data_field_female_pop_10_to_14_years.deleted = '0')
WHERE (( (de_node.type IN  ('census')) ))
LIMIT 10 OFFSET 0

-Jesse

Anybody’s picture

Issue summary: View changes

This is a very important case and views "core developers" should have a look at it.
I've just run into this issues problem and it just makes it impossible to use views for such a case though.

The patch above works for me.
How can we get this forward?

mbaynton’s picture

+1, please look at including.

Chris Matthews’s picture

The 6 year old patch in #1 to handlers.inc, view.inc and views_plugin_query_default.inc does not apply to the latest views 7.x-3.x-dev and if still relevant needs to be rerolled.

Checking patch includes/handlers.inc...
error: while searching for:
   *   The source query, implementation of views_plugin_query.
   */
  function build_join($select_query, $table, $view_query) {
    if (empty($this->definition['table formula'])) {
      $right_table = $this->table;
    }
    else {
      $right_table = $this->definition['table formula'];
    }


error: patch failed: includes/handlers.inc:1523
error: includes/handlers.inc: patch does not apply
Checking patch includes/view.inc...
error: while searching for:
  var $base_database = NULL;

  /**
   * Here comes a list of the possible handler which are active on this view.
   */


error: patch failed: includes/view.inc:202
error: includes/view.inc: patch does not apply
Checking patch plugins/views_plugin_query_default.inc...
error: while searching for:

    // Go ahead and build the query.
    // db_select doesn't support to specify the key, so use getConnection directly.
    $query = Database::getConnection($target, $key)
      ->select($this->base_table, $this->base_table, $options)
      ->addTag('views')
      ->addTag('views_' . $this->view->name);


error: patch failed: plugins/views_plugin_query_default.inc:1290
error: plugins/views_plugin_query_default.inc: patch does not apply