Views allows modules to define tables that are aliases of other tables. This can be useful when one needs to join a table against itself. For example, the bio module copies the existing node table, aliases it to "bio", and then joins against the node table on uid (e.g. node node LEFT JOIN node bio ON node.uid = bio.uid AND bio.type = 'bio').

This allows bio to reuse all the handlers in views_node.inc instead of re-implementing them itself. Some of these handlers, however, are not table alias safe, and assume that the only important table is 'node'. For example, in views_handler_field_nodelink, the hander creates a link to the node based on $data->nid. This is incorrect for the bio aliased table where it should be creating the link based on $data->bio_nid

The attached patch makes all the views handlers and sorters alias safe. Each handler and sorter computes the field name that it needs to work on, instead of just assuming node_foo. This patch introduces a helper function to compute the nid from the current table, as this was a common task.

This patch also adds some addlfields to certain node table fields. Mostly, this is just adding the nid for aliased tables.

Finally, there are two areas that deserve more attention:
1. In views_handler_filter_distinct I have removed the call to $query->set_distinct. Set distinct always works on node.nid - not any other table aliases.Having the GROUP BY on tablename.nid appears to be sufficient. Is there a need for the SQL DISTINCT statement?
2. I could not make the views_handler_filter_isnew generic to other table aliases. I have made a copy for bio module that uses the bio aliased tables. I think this will be necessary for any modules copying the node table if they wish to support "isnew" on their aliases. I would like to see the function made generic, but let's not wait for that if the rest of this patch is ready.

While this is a big patch, all the changes are fairly minor and will be extremely helpful for modules joining an aliased node table against the leftmost node table.

CommentFileSizeAuthor
#7 views_node_aliases_fields.patch12.02 KBmfredrickson
#2 views_node_aliases_nid.patch9.65 KBfago
#1 views_node_aliases.patch8.15 KBfago
views_node_inc_20070509_node_alias_safe.patch8.81 KBmfredrickson
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
FileSize
views_node_aliases.patch8.15 KB

great work! this also solves the problems of some of these fields with views fusion, e.g. the node title link uses now always the correct table!

@views_handler_filter_distinct:
I also think that distinct isn't really needed there. I've attached an updated patch: I've removed your comment as well as the check for $query->no_distinct - as it isn't needed any more. Note that I've also tested summary views, of course they are still working with this modifications.

@views_handler_filter_isnew: yeah that's a hard one.. ;) We could use add_table with custom joininfo to link back to the correct node table - but copying this single function doesn't harm either.

Note: The attached patch is the same as yours, apart from the changed filter Node: distinct.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
FileSize
views_node_aliases_nid.patch9.65 KB

I thought a bit more about this. It's a common problem of views_fusion that some fields make queries in their field handler, but use the wrong node id for that, as they just use $data->nid.

So, I propose to introduce a nice little views helper function, that returns the appropriate node id for all cases. I've started with your proposed _views_handler_field_node_find_nid() and added views_field_get_node_id() to views.module.
This handles not only the case of reused views definitions such as bio does or will do, but also the case of fused views while falling back to $data->nid.

I've done a quick scan through the views definitions, and I found that the taxonomy field handlers also use $data->nid.

So attached is a new patch, which
* makes view_node.inc handler table alias safe (such as the above patches)
* introduces the views_field_get_node_id() API function
* lets the taxonomy field handler use this function (which fixes them if they are used for fused views)

@mfredrickson: It would be cool if you could give this another test with your bio views definitions - it should just work like with your patch.

mfredrickson’s picture

mfredrickson CreditAttribution: mfredrickson commented

I've tested the followup patch from Fago, and it looks good. All the existing functionality continues to work.

On the vocab stuff, that is actually not working. I'm going to trouble shoot and dig a little deeper. After we nail that, I think we can assign this RTBC and get Earl's attention.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

thanks fort testing!

The vocab/taxonomy stuff won't work if you just reuse the definitions.
The problem with it is, that it can't know with which node table you joined it. For that we would have to go through all the joininfo and look how the table got joined - not so easy I think...

In contrast it does work with views_fusion, because it just uses the information of the views node table, in which the field resides. Each view of the fusion is identified with its table alias -> so just use $alias . 'node' and you have the right node table...

So a short conclusion:
views_field_get_node_id() gets the node id of
* the node table of which the field resides in (useful for node table field handlers reusing like you proposed)
* the node table of the view, in which the field has been defined in case of fused views
* the usual node table aka primary table else

mfredrickson’s picture

mfredrickson CreditAttribution: mfredrickson commented

I think it would be possible to look back through the views table definitions and find the closest table with the requested field. The function signature might look like:

function views_find_closest_field($current_table, $field_name)

The algorithm would be something like

  1. Get the views table definitions ($tables = views_get_tables())
  2. Look up $tables[$current_table]
  3. Does the current table def contain field $field_name? If so, return $current_table
  4. If not, find the table $current_table joins on ($next_table = $tables[$current_table]['join']['left'])
  5. return views_find_closest_field($next_table, $field_name)
  6. Insert a catch that if we hit 'node', terminate

This would find the closest 'nid' column that is exposed as in the views table definitions. In the case of the taxonomy query, this will give the correct nid column for an aliased node table.

I'll implement this idea and post back a new patch.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

nice idea - that will do it for most cases.

However, if one uses add_table() with custom joininfo it would fail. But I think we can live with that.

mfredrickson’s picture

mfredrickson CreditAttribution: mfredrickson commented
Status: Needs review » Reviewed & tested by the community
FileSize
views_node_aliases_fields.patch12.02 KB

I've implemented that function and refactored views_field_get_node_id to rely on it.

I'm going to mark this patch RTBC as it has undergone extensive testing at this point and think the idea is sound.

There is one outstanding caveat, that I think we can just live with for now: views always adds the nid to the query. But when other tables alias node, alias.nid is not automatically added to the query. Therefore, to find the appropriate nid for aliased tables, we need to load the nid manually in some manner -- either with a NID field included or another field that has 'nid' in it's $addlfields array.

Since this is such an edge case, let's document that behavior for now and move on. For bios I may write a hook_views_alter that always adds the bio.nid if a bio_* field is in the query.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
Status: Reviewed & tested by the community » Needs work

hm, I've tried it on site with views fusion and it badly failed, when I tried viewing a fused view -> Error 500.
I suppose it's getting in an endless loop for this case.

I had no time for a close look, but I think we've to either fix it to work in any case or we revert to the not as powerful, but safe working variant from #2.

mfredrickson’s picture

mfredrickson CreditAttribution: mfredrickson commented
Status: Needs work » Reviewed & tested by the community

Ok. Let's revert to Fago's patch in #2. I've reviewed it and I think it's solid.

I'll continue to work on the find_closest_field function. I suspect the problem is that views fusion changes the $view->primary_table. Since my patch would always terminate on the 'node' definition, it's possible that changing the primary_table would break the assumption that 'node' is the root of every view.

DesignWork’s picture

DesignWork CreditAttribution: DesignWork commented

Hi mfredrikson, Hi fargo,

i tested the 2# patch from fargo. But i still have a problem joining two nodes with fusion. I´m using a kind of bio.module for my user pages now i´m trying to geht the postalcode and a taxonomy term together. But its not working.

I can use whatever CCK field but it will not work. I got this statement: 


user warning: Unknown column 'node_data_field_name.field_name_value' in 'where clause' query: SELECT node.nid, node_data_field_arbeitsgebiet.field_arbeitsgebiet_value AS node_data_field_arbeitsgebiet_field_arbeitsgebiet_value, v72node.nid AS v72node_nid, v72node_data_field_vorname.field_vorname_value AS v72node_data_field_vorname_field_vorname_value, v72node_data_field_name.field_name_value AS v72node_data_field_name_field_name_value, v72location.postal_code AS v72location_postal_code FROM node node LEFT JOIN term_node term_node ON node.nid = term_node.nid LEFT JOIN term_hierarchy term_hierarchy ON term_node.tid = term_hierarchy.tid LEFT JOIN content_type_bild node_data_field_arbeitsgebiet ON node.vid = node_data_field_arbeitsgebiet.vid INNER JOIN nodefamily nodefamily_child ON node.nid = nodefamily_child.child_nid LEFT JOIN node v72node ON nodefamily_child.parent_nid = v72node.nid LEFT JOIN content_type_kontakt_daten v72node_data_field_name ON v72node.vid = v72node_data_field_name.vid LEFT JOIN content_type_kontakt_daten v72node_data_field_vorname ON v72node.vid = v72node_data_field_vorname.vid LEFT JOIN location v72location ON v72node.vid = v72location.eid WHERE (term_node.tid = '29') AND (UPPER(node_data_field_name.field_name_value) = UPPER('Dirk')) AND (v72node.type IN ('kontakt_daten')) in /kunden/163565_20459/webseiten/dirksway/includes/database.mysql.inc on line 172.

Any suggestion how to resolve it? http://www.freelens.ws/dirksway/rg_bild

Dirk

PS. Drupal 5.1, and the latest releases from views an viewsfusion, cck etc.....

mfredrickson’s picture

mfredrickson CreditAttribution: mfredrickson commented

I think this is a bug in views fusion, not this patch or views. Notice that there is a table alias called v72node_data_field_name. Viewsfusion prefixes table names, and the handler that is trying to insert the filter in the where clause is using the original table alias.

Patch continues to be RTBC.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

yes, designwork is talking about another issue, which doesn't belong here.

It isn't related to this patch. So let's get it in!

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

This will go in after Views 1.6.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

we are now "after views 1.6". maybe in views2?

esmerel’s picture

esmerel CreditAttribution: esmerel commented
Status: Reviewed & tested by the community » Closed (won't fix)

At this time, only security fixes will be made to the 5.x version of Views.