Views generates a bad SQL/JOINS in the following case:
1. Have 2 nodes types "Node_Type_A" and "Node_Type_B"
2. "B" contains an imagefield + title
3. Have 2 nodes of NodeType_B (nidB1 and nidB2)
4. Have 1 node of Node_Type_A (nidA3)
5. Have nodA3 contains 2 references to NodeType_B (to nodes nidB1 and nidB2)
6. In the view, you add 2 imagefields from NodeType_B (from the two related nodes nidB1 and nidB2)

Result:
The displayed result returns twice the information from nidB2 also for nidB3

Exported view:

$view = new view;
$view->name = 'view_couples';
$view->description = 'Listing of couples';
$view->tag = 'couples';
$view->view_php = '';
$view->base_table = 'node';
$view->is_cacheable = FALSE;
$view->api_version = 2;
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
$handler = $view->new_display('default', 'Default Gallery View', 'default');
$handler->override_option('relationships', array(
  'field_partner_m_nid' => array(
    'label' => 'Partner',
    'required' => 1,
    'delta' => -1,
    'id' => 'field_partner_m_nid',
    'table' => 'node_data_field_partner_m',
    'field' => 'field_partner_m_nid',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
  'field_partner_f_nid' => array(
    'label' => 'Partner(lady)',
    'required' => 1,
    'delta' => -1,
    'id' => 'field_partner_f_nid',
    'table' => 'node_data_field_partner_f',
    'field' => 'field_partner_f_nid',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
));
$handler->override_option('fields', array(
  'field_teaser_img_fid' => array(
    'label' => '',
    'link_to_node' => 0,
    'label_type' => 'none',
    'format' => 'Portrait_linked',
    'multiple' => array(
      'group' => TRUE,
      'multiple_number' => '',
      'multiple_from' => '',
      'multiple_reversed' => FALSE,
    ),
    'exclude' => 0,
    'id' => 'field_teaser_img_fid',
    'table' => 'node_data_field_teaser_img',
    'field' => 'field_teaser_img_fid',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'field_partner_m_nid',
  ),
  'field_teaser_img_fid_1' => array(
    'label' => '',
    'link_to_node' => 0,
    'label_type' => 'none',
    'format' => 'Portrait_linked',
    'multiple' => array(
      'group' => TRUE,
      'multiple_number' => '',
      'multiple_from' => '',
      'multiple_reversed' => FALSE,
    ),
    'exclude' => 0,
    'id' => 'field_teaser_img_fid_1',
    'table' => 'node_data_field_teaser_img',
    'field' => 'field_teaser_img_fid',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'field_partner_f_nid',
  ),
  'title' => array(
    'label' => 'Title',
    'link_to_node' => 1,
    'exclude' => 0,
    'id' => 'title',
    'table' => 'node',
    'field' => 'title',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
  'field_date_start_value2' => array(
    'label' => '',
    'link_to_node' => 0,
    'label_type' => 'widget',
    'format' => 'short',
    'multiple' => array(
      'group' => TRUE,
      'multiple_number' => '',
      'multiple_from' => '',
      'multiple_reversed' => FALSE,
    ),
    'exclude' => 0,
    'id' => 'field_date_start_value2',
    'table' => 'node_data_field_date_start',
    'field' => 'field_date_start_value2',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
  'field_category_stt_value' => array(
    'label' => 'STT',
    'link_to_node' => 0,
    'label_type' => 'custom',
    'format' => 'default',
    'multiple' => array(
      'group' => TRUE,
      'multiple_number' => '',
      'multiple_from' => '',
      'multiple_reversed' => FALSE,
    ),
    'exclude' => 0,
    'id' => 'field_category_stt_value',
    'table' => 'node_data_field_category_stt',
    'field' => 'field_category_stt_value',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
  'field_category_lat_value' => array(
    'label' => 'LAT',
    'link_to_node' => 0,
    'label_type' => 'custom',
    'format' => 'default',
    'multiple' => array(
      'group' => TRUE,
      'multiple_number' => '',
      'multiple_from' => '',
      'multiple_reversed' => FALSE,
    ),
    'exclude' => 0,
    'id' => 'field_category_lat_value',
    'table' => 'node_data_field_category_lat',
    'field' => 'field_category_lat_value',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
));
$handler->override_option('sorts', array(
  'field_date_start_value' => array(
    'order' => 'ASC',
    'id' => 'field_date_start_value',
    'table' => 'node_data_field_date_start',
    'field' => 'field_date_start_value',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
));
$handler->override_option('filters', array(
  'status' => array(
    'operator' => '=',
    'value' => 1,
    'group' => '0',
    'exposed' => FALSE,
    'expose' => array(
      'operator' => FALSE,
      'label' => '',
    ),
    'id' => 'status',
    'table' => 'node',
    'field' => 'status',
    'relationship' => 'none',
  ),
  'type' => array(
    'operator' => 'in',
    'value' => array(
      'cck_couple' => 'cck_couple',
    ),
    'group' => '0',
    'exposed' => FALSE,
    'expose' => array(
      'operator' => FALSE,
      'label' => '',
    ),
    'id' => 'type',
    'table' => 'node',
    'field' => 'type',
    'relationship' => 'none',
    'override' => array(
      'button' => 'Override',
    ),
  ),
));
$handler->override_option('access', array(
  'type' => 'none',
));
$handler->override_option('title', 'Galéria párov');
$handler->override_option('header_format', '1');
$handler->override_option('header_empty', 0);
$handler->override_option('empty', 'No items have been found matching your criteria');
$handler->override_option('empty_format', '1');
$handler->override_option('use_ajax', TRUE);
$handler->override_option('items_per_page', 16);
$handler->override_option('use_pager', 'mini');
$handler->override_option('style_plugin', 'table');
$handler->override_option('style_options', array(
  'grouping' => '',
  'override' => 1,
  'sticky' => 0,
  'order' => 'asc',
  'columns' => array(
    'field_teaser_img_fid' => 'field_teaser_img_fid',
    'field_teaser_img_fid_1' => 'field_teaser_img_fid_1',
    'title' => 'title',
    'field_date_start_value2' => 'title',
    'field_category_stt_value' => 'field_category_stt_value',
    'field_category_lat_value' => 'field_category_lat_value',
  ),
  'info' => array(
    'field_teaser_img_fid' => array(
      'separator' => '',
    ),
    'field_teaser_img_fid_1' => array(
      'separator' => '',
    ),
    'title' => array(
      'sortable' => 0,
      'separator' => '<br/>',
    ),
    'field_date_start_value2' => array(
      'sortable' => 0,
      'separator' => '',
    ),
    'field_category_stt_value' => array(
      'sortable' => 0,
      'separator' => '',
    ),
    'field_category_lat_value' => array(
      'sortable' => 0,
      'separator' => '',
    ),
  ),
  'default' => '-1',
));
$handler = $view->new_display('page', 'Page', 'page_1');
$handler->override_option('path', 'gallery');
$handler->override_option('menu', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));
$handler->override_option('tab_options', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));

SQL Select generated by Views:

SELECT node.nid AS nid,
   node_data_field_teaser_img.field_teaser_img_fid AS node_data_field_teaser_img_field_teaser_img_fid,
   node_data_field_teaser_img.field_teaser_img_list AS node_data_field_teaser_img_field_teaser_img_list,
   node_data_field_teaser_img.field_teaser_img_data AS node_data_field_teaser_img_field_teaser_img_data,
   node_data_field_teaser_img.nid AS node_data_field_teaser_img_nid,
   node_node_data_field_partner_m.type AS node_node_data_field_partner_m_type,
   node_node_data_field_partner_f.type AS node_node_data_field_partner_f_type,
   node.title AS node_title,
   node_data_field_date_start.field_date_start_value AS node_data_field_date_start_field_date_start_value,
   node_data_field_date_start.field_date_start_value2 AS node_data_field_date_start_field_date_start_value2,
   node_data_field_date_start.nid AS node_data_field_date_start_nid,
   node.type AS node_type,
   node_data_field_category_stt.field_category_stt_value AS node_data_field_category_stt_field_category_stt_value,
   node_data_field_category_stt.field_category_stt_format AS node_data_field_category_stt_field_category_stt_format,
   node_data_field_category_stt.nid AS node_data_field_category_stt_nid,
   node_data_field_category_stt.field_category_lat_value AS node_data_field_category_stt_field_category_lat_value,
   node_data_field_category_stt.field_category_lat_format AS node_data_field_category_stt_field_category_lat_format
 FROM drupal6_node node 
 LEFT JOIN drupal6_content_type_cck_couple node_data_field_partner_m ON node.vid = node_data_field_partner_m.vid
 INNER JOIN drupal6_node node_node_data_field_partner_m ON node_data_field_partner_m.field_partner_m_nid = node_node_data_field_partner_m.nid
 LEFT JOIN drupal6_content_type_cck_couple node_data_field_partner_f ON node.vid = node_data_field_partner_f.vid
 INNER JOIN drupal6_node node_node_data_field_partner_f ON node_data_field_partner_f.field_partner_f_nid = node_node_data_field_partner_f.nid
 LEFT JOIN drupal6_content_type_cck_std_node node_data_field_teaser_img ON node_node_data_field_partner_f.vid = node_data_field_teaser_img.vid
 LEFT JOIN drupal6_content_field_date_start node_data_field_date_start ON node.vid = node_data_field_date_start.vid
 LEFT JOIN drupal6_content_type_cck_couple node_data_field_category_stt ON node.vid = node_data_field_category_stt.vid
 WHERE (node.status <> 0) AND (node.type in ('cck_couple'))
   ORDER BY node_data_field_date_start_field_date_start_value ASC
CommentFileSizeAuthor
#17 ensure_my_table_multiple.patch617 bytesquicksketch

Comments

merlinofchaos’s picture

Status: Active » Postponed (maintainer needs more info)

I'm afraid I can't duplicate this. I don't have imagefield installed on my setup, so I only used titles, but when I set this up it works correctly. If it's the imagefield that is causing the problem, file this ticket against imagefield module (or maybe CCK).

notabenem’s picture

Status: Postponed (maintainer needs more info) » Closed (fixed)

You're right. With titles and nids it works as expected. However, I don't see, why should be the culprit imagefield, if the generated SQL contains only 1 set of fields related to "teaser_img", though in the definition of the view it is clear, that two set of such fields should have been generated to the SQL.

notabenem’s picture

Title: Bad joins with NodeReferrer and Relations » Bad joins with NodeReferrer and Views Relations
Project: Views (for Drupal 7) » FileField
Version: 6.x-2.0-rc4 » 6.x-3.0-alpha5
Component: Views Data » Code
Status: Closed (fixed) » Active

Reopening for Filefield.

dopry’s picture

Status: Active » Postponed (maintainer needs more info)

initial guess some sort of join issue, but I don't really have time to dig deeply into views/cck/field integration at the moment. Maybe you have time to do some digging?

notabenem’s picture

@dopry: i am not really a drupal coder, but if you give me some good starting points, I may be able to pinpoint the culprit.

notabenem’s picture

So far, I discovered that one LEFT JOINS and a couple of columns in the SELECT is missing. Specifically, albeit

LEFT JOIN drupal6_content_type_cck_std_node node_data_field_teaser_img ON node_node_data_field_partner_f.vid = node_data_field_teaser_img.vid

is included in the SQL query, an similar JOIN is missing for node_node_data_field_partner_m. It should be something like below:

LEFT JOIN drupal6_content_type_cck_std_node node_data_field_teaser_img_2 ON node_node_data_field_partner_m.vid = node_data_field_teaser_img_2.vid

In addition, a whole set of columns is missing, as outlined below (or something similar, that would correspond with the missing JOIN):

   node_data_field_teaser_img_2.field_teaser_img_fid AS node_data_field_teaser_img_field_teaser_img_fid_2,
   node_data_field_teaser_img_2.field_teaser_img_list AS node_data_field_teaser_img_field_teaser_img_list_2,
   node_data_field_teaser_img_2.field_teaser_img_data AS node_data_field_teaser_img_field_teaser_img_data_2,
   node_data_field_teaser_img_2.nid AS node_data_field_teaser_img_nid_2,

Which part of the code is where such information is defined? Is it in filefield or imagefield? Which file exactly?

notabenem’s picture

Version: 6.x-3.0-alpha5 » 6.x-3.0-alpha6

Still applies to the latest release, alpha6.

dopry’s picture

Project: FileField » ImageField
Version: 6.x-3.0-alpha6 » 6.x-3.x-dev

is this views 1 or views 2? I use views 2 with imagefield without issue.. maybe you need to add the relationship? does it also apply to -dev?

notabenem’s picture

Hi dopry,
This is using views 2.3 (tried today with the newest version) and also filefield a7 + imagefield a4.

I really think the issue will be somewhere with the 2nd relationship. because as soon as I add another relationship, columns that would normally retrieve their value using the 1st relationship CHANGE their value as if they used the 2nd or 3rd, i.e. they're all using the LAST added relationship. In the SQL this shows up as a single JOIN clause that is relevant for the LAST relationship, but all of the columns are using it.

It also applies to the the latest dev (6.2.2009) of filefield. If you're willing, we could set up a webmeeting or crossloop or whatever needed for you to see it on my deskstop and maybe debug it.

quicksketch’s picture

Could you try out the latest dev versions of ImageField and FileField? It's likely this problem was fixed in #397578: Uncouple ImageField from FileField Custom Hooks.

notabenem’s picture

quicksketch: Unfortunately, this still does not work. How can I help you to pinpoint the problem?

quicksketch’s picture

notabenem, it'd be a huge help if I could actually get a working Drupal site that has all this configuration setup already. It looks like it'll take quite a bit of setup to reproduce the problem. Could you provide a SQL dump of the site you're experiencing this on (with all important information removed) or setup a site with no content but has these content types and views setup?

notabenem’s picture

quicksketch: once you manage to set up the site, look for view: 'gallery_adv'. The last columns of this view are the ones exhibiting the bug.

PGiro’s picture

I also ran into this problem with the latest versions of views, imagefield and filefield.

The same thing happens if you create a view of "comments" and try to display an image field on the containing node

quicksketch’s picture

Status: Postponed (maintainer needs more info) » Active

notabenem, I finally got a chance to look at the site you sent over. This is definitely a real, but difficult problem. I thought it might have something to do with the CCK implementation, but oddly other CCK fields (such as text) work just fine when the same field is pulled in twice from the 2 different referenced nodes.

I'll do my best to try to track down this problem, though I'm not sure it's in FileField (which just uses the same Views integration as other CCK modules). I'm most suspecting CCK's Views implementation, but it's too early to tell where this is really coming from.

quicksketch’s picture

Alright the first problem I've found is that we have two "foreign keys" in a single table (the two node reference fields within a single node type). When CCK adds these fields to the view, it doesn't give each one a unique table name. Instead both relationships use the same table alias, causing only one of the relationships to have an effect.

Interestingly, this means you can get around the problem by making one (or both) of the fields support multiple values. Of course this isn't optimal but it fixes the problem I believe. I'll see what we can do to ensure CCK gives each table a unique alias when using node references.

quicksketch’s picture

Title: Bad joins with NodeReferrer and Views Relations » ensure_my_table() Limited to One Relationship Per Table
Project: ImageField » Views (for Drupal 7)
Version: 6.x-3.x-dev » 6.x-2.x-dev
Status: Active » Needs review
StatusFileSize
new617 bytes

Okay! I think I've found the source of the problem. Turns out it could be fixed in CCK, but I think solving this in Views would be the best solution, since it'll fix the same problem that other modules might have (rare as it might be).

As I suggested above, the problem is that table aliases are only being created once per node reference relationship. CCK currently uses the default views_handler_argument::query() method for its queries which contains the following code:

  function query() {
    $this->ensure_my_table();
    $placeholder = empty($this->definition['numeric']) ? "'%s'" : '%d';
    $this->query->add_where(0, "$this->table_alias.$this->real_field = $placeholder", $this->argument);
  }

Inside of ensure_my_table(), eventually queue_table() is called to add the field's table. Unfortunately queue_table() doesn't check which relationship the table is in when it creates the table's alias. The end result is that you can only have only relationship a single relationship from one table to another table, additional relationships are effectively ignored.

To reproduce:
- Install Views, Content, Select, Node Reference, and Text Modules.
- Create a content type "multi_reference".
- Add 2 node reference fields (field_ref_a and field_ref_b) that are able to reference story content.
- Add a CCK text field (field_textfield) to the story node type.
- Create 2 pieces of story content, filling out the field_textfield value in each one.
- Create a single multi_reference piece of content, and fill out field_ref_a with the first story and field_ref_b with the second story.
- Create a new node view. Add a relationship for field_ref_a and field_ref_b.
- Filter the view to just Node: type == 'multi_reference'
- Add the field_textfield twice to the view, once using the field_ref_a relationship and once with the field_ref_b relationship.

Expect result:
- The fields contain two different values from the first and second story nodes.

Actual result:
- Both fields contain the same value from the first story node.

I could go on and explain this further, but I think knowing how to reproduce the problem and looking at the patch is probably going to be more productive. notabenem, could you apply this patch to Views and ensure that it works on your installation?

merlinofchaos’s picture

quicksketch: Maybe we shouldn't prepend the relationship if the relationship happens to == $this->base_table? Which means that there is effectively no relationship.

I'm not sure if we can safely just check that, though. Still I think this will fix a couple of other bugs I've seen.

quicksketch’s picture

Thanks for taking a look at this, the more I typed the more complicated this sounded. :P

I haven't gotten deep enough to know when which variables are populated, but I know that when no relationship is defined the alias just matches the Views table name and this conditional doesn't fire at all. It doesn't seem like there'd be any harm in adding the relationship name even if it was the same as the base_table though, other than making our aliases unnecessary longer. The situation where that would come up seems like it would be even more unusual than this problem, which is already pretty fringe.

carlosg2’s picture

@quicksketch

Your patch solves the problem.
Look at http://drupal.org/node/428322

Many thanks

notabenem’s picture

@quicksketch: You're the MAN! Issue solved. Thanks a lot of times!
PS: It's funny to see how this ticket traveled round several modules just to return to the first one it was filed against, views :)

karens’s picture

Subscribing, this may affect some of my modules so I need to check.

merlinofchaos’s picture

Status: Needs review » Fixed

Committed! I went ahead and checked to see if it relationship is base_table so we don't do extra, unnecessary appending. I think that should continue to work just fine.

quicksketch’s picture

Awesome, thanks merlinofchaos! This should fix quite a few support requests in the Flag queue.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.