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
Comments
Comment #1
merlinofchaos commentedI'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).
Comment #2
notabenem commentedYou'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.
Comment #3
notabenem commentedReopening for Filefield.
Comment #4
dopry commentedinitial 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?
Comment #5
notabenem commented@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.
Comment #6
notabenem commentedSo 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.vidis 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.vidIn addition, a whole set of columns is missing, as outlined below (or something similar, that would correspond with the missing JOIN):
Which part of the code is where such information is defined? Is it in filefield or imagefield? Which file exactly?
Comment #7
notabenem commentedStill applies to the latest release, alpha6.
Comment #8
dopry commentedis 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?
Comment #9
notabenem commentedHi 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.
Comment #10
quicksketchCould 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.
Comment #11
notabenem commentedquicksketch: Unfortunately, this still does not work. How can I help you to pinpoint the problem?
Comment #12
quicksketchnotabenem, 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?
Comment #13
notabenem commentedquicksketch: 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.
Comment #14
PGiro commentedI 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
Comment #15
quicksketchnotabenem, 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.
Comment #16
quicksketchAlright 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.
Comment #17
quicksketchOkay! 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:
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?
Comment #18
merlinofchaos commentedquicksketch: 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.
Comment #19
quicksketchThanks 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.
Comment #20
carlosg2 commented@quicksketch
Your patch solves the problem.
Look at http://drupal.org/node/428322
Many thanks
Comment #21
notabenem commented@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 :)
Comment #22
karens commentedSubscribing, this may affect some of my modules so I need to check.
Comment #23
merlinofchaos commentedCommitted! 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.
Comment #24
quicksketchAwesome, thanks merlinofchaos! This should fix quite a few support requests in the Flag queue.