yched passed me this code, which needed a little polishing (to create summaries) and asked me to submit as a patch. It seems to be working fine - both for normal listings (i.e. pass in an node ID argument and views returns all the nodes that that node references) and for summaries (which list all nodes that refer to any other node using this field - easy to get your forward/reverse muddled up here!).

Could use a read through from a views-head, to check we are doing things in the best way.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

BY default CCK adds no indexes beyond the primary key of each table. So a think this is going to be slow. Would be great if we added an index on the other side of node and user refs. Why not in this patch?

Owen Barton’s picture

That's a good idea indeed - well spotted! We would need to add them on both the 'nid' and 'field_*_nid' columns I think, since they are both used by views here. Anywhere else?

KarenS’s picture

If you can re-roll the patch with the indexes, I'll get it committed. Thanks!

Owen Barton’s picture

Here we are. I have kept things as simple as possible for now - you can only index the field provided column, and it automatically adds an additional index for the nid whenever any existing column has an index, on the basis that it is likely this nid will be involved in views queries (this view, at least). We could obviously get much more fine grained here, but this feels like a sufficient solution for Drupal 5. I have tested that indexes work on single and multiple/shared fields.

For Drupal 6, schema makes a lot of this easy, so I think we can keep the same simple attribute to index just the field itself, but also add an $op 'index' on hook_field_settings, which would allow you to add an index or 2 to other fields (including multi-column indexes and all that good stuff). Ideally we could also make it smart, as suggested on http://groups.drupal.org/node/7614 by linking the index with actual usage of a particular views argument or filter.

reubenavery’s picture

These patches won't work against CCK 5.x-1.7 ..

Looking at the dates on this thread, I would have thought that 1.7 would have this incorporated, but it doesn't.

I desperately need reverse nodereference views arguments!!!

reubenavery’s picture

Hello again,

Well I rolled back to CCK 1.6 and got this patch to apply correctly. However, I'm afraid there is a bad bug here with nodes which have revisions-- it basically returns all nodereference joins for the node all the way backwards. I suspect this is because the join is being made via node.nid = {content_field_nodereference}.nid, when what it needs to be on is the vid column.

I tried to fix this myself in the patch but my drupal knowledge failed me ;-)

reubenavery’s picture

Status: Needs review » Needs work

see above

oda’s picture

FileSize
2.77 KB

Hi, I need transparent reverse and editable node reference, so I wrote this code. I'm almost sleeping on my keyboard, said that, sorry about the bad code... I know that this patch is not perfect but I think it works (at least it works for me ;) I hope it helps you.

Grantovich’s picture

Is this going anywhere? I'll give it a bump because KarenS mentioned getting it committed, and I would very much like to have the functionality. In fact, it seems to me like this is really the "forward" way of doing it, since you are just displaying the nodes that the user typed in; if anything the existing behavior is "reverse" (or at least the opposite of how I expected it to work).

Owen Barton’s picture

Status: Needs work » Needs review
FileSize
6.29 KB

Here is an updated patch (to the latest 5.x dev branch).

This also fixes the vid issue that reubidium found (we just need to join on the latest vid).

Owen Barton’s picture

Can anyone help review and test this, it would be really nice if we could get this in before it gets (too!) out of sync again, because it's a fairly deep topic to get your head around each time (and retest). :)

Owen Barton’s picture

Updating patch to latest 5 branch

electricmonk’s picture

I'm getting this message when using with CCK 5.x-1.7:

user warning: Duplicate key name 'nid' query: CREATE INDEX nid ON content_type_page (nid) in D:\dev\sandbox\drupal-5.10\includes\database.mysqli.inc on line 154.

This is caused by the change to rows 1380-1383 on content_admin.inc and seems to me like wrong usage of the CREATE INDEX clause; the current code is trying to create an index named 'nid', which conflicts with the 'nid' column. I've prefixed it with 'ix_', which seems to work, but I don't feel confident enough to submit my own code.

Owen Barton’s picture

Here is a patch rerolled for the latest 5.x-dev changes, and including adding "_idx" suffixes (in line with other CCK code) to generated indexes, which I have tested and appear to be working as they should.

Tests and review welcome!

Owen Barton’s picture

Title: Reverse node-reference views argument » Reverse node-reference views argument with appropriate index management
FileSize
6.52 KB

Patch (and making title more descriptive of scope)

Bevan’s picture

FileSize
6.28 KB

Used and tested on DrupalSouth.net.nz. The title op of the views argument handler was broken. I rerolled the patch with the fix, so that "%1" can be replaced with the node title in the title of the view when the reverse node reference is used as an argument.

Bevan’s picture

FileSize
6.3 KB

Merging my patch in with Grugnog's latest

tostinni’s picture

Is this have been solved in CCK 6.x-2.x-dev ?
I'm wondering if it's not the same kind of problem as I was having regarding generating a list of users referenced in a node.
http://drupal.org/node/317936 and http://lists.drupal.org/pipermail/support/2008-October/010022.html

So as I understand, the feature is missing from CCK which doesn't expose reversed node-ref and user-ref fields and tables to views ? Am I correct on this ?

Bevan’s picture

tostinni, this patch implements the missing reverse node reference feature you queried about, but not the reverse user reference.

tostinni’s picture

Thanks Bevan, but I was wondering, there's a patch for 5.x which is feature frozen and also one for 6.x should we bumped the version to 6x-2.x-dev ?

Also I think I'd have to study this patch in order to come with a similar one for user reference ;)

Owen Barton’s picture

This patch is for Drupal 5 right now. We can port to Drupal 6, but it would be good to get a nod from yched or Karen first.

KarenS’s picture

Sure, if you've got time, please try to port it to D6. It will be quite different in D6 because of all the Views 2 changes.

I'm not doing anything with D5 right now, focused on getting D6 out. But if the D5 version is reliably working and others can confirm that, I can commit it.

Bevan’s picture

I think another one or two reviews are necessary before this is RTBC

yched’s picture

Note that relationships in Views 2 make this trivial as long as you're using a 'Field' row-style View :
Add a 'Node:id' argument, add a relationship on the noderef field, and add the node title (for instance) through the relationship.
You get the titles of the nodes referenced by the nid you provide as an argument.

This does not work for 'Node' row-style : the nodes that gets displayed are always the ones of the main 'nid', there's no way to tell Views to follow a relationship instead.
I wonder if that would be a valid feature request for Views.

moshe weitzman’s picture

Don't request that feature! It might unravel the very fabric of the space-time continuum.

@yched - Views2 does not help with the "index management" part of this issue. Not sure if we have a different issue open specific to indices.

jmiccolis’s picture

Status: Needs review » Needs work

I've tested the patch from #17 and there is an issue when you add two node-reference fields to the same content-type. It looks like a test needs to be added to prevent ensure the 'nid_idx' index is added only once.

The specific error is:

user warning: Duplicate key name 'nid_idx' query: CREATE INDEX nid_idx ON content_type_one (nid) in /Library/WebServer/Documents/drupal-5.12/includes/database.mysql.inc on line 174.

I've been increasing wishing cck had the ability to add indexes lately, so I can't wait to see this go in. :)

yched’s picture

First of all, my sincere apologies to Grugnog2 for letting this drop off my radar, after initially asking him to turn this into a proper patch.

To summarize my personnal position on this.
The patch now has two features :
1) reversed noderef arg,
2) index on cck data columns

I don't think those two features should be linked.
1) does outline the lack of 2), but AFAICT just like any other Views filter, arg or sort on a CCK field, not more.
2) is a long-time lacking feature. It's complex and fully deserves its own patch.

So, taking each issue separately :
1) reversed noderef arg :This is a non-issue in D6 / Views 2 using relationships. The only current limitation is with 'Node' row style, but this should be addressed in Views and I opened #327366: Let row plugins follow relationship for this (Earl thinks it's completely reasonable, more of an overlook actually).
I fully support adding this to D5 as it exists in the current patch. Giving it a quick try, it seems to work fine except for nodes for which the noderef field is empty (also generates a broken link in the summary). Thus keeping as 'code needs work'

2) index on cck columns : to ensure upgradability (and a non-nightmarish maintanance of the D6 upgrade path), this should be done in D6 first before a D5 backport can be considered.
Big -1 on committing a solution that creates indexes systematically, even if only for columns that are marked 'indexable' in hook_field_settings('columns').
More generally, I suggest discussion on this goes on in #231453: Allow indexing columns

yngvewb’s picture

I can confirm that this patch works. In a view you add the argument "Referenced by a certain field".

Darren Oh’s picture

Version: 5.x-1.x-dev » 6.x-2.x-dev
Status: Needs work » Needs review
FileSize
4.43 KB

This is still an issue for Views 2. Attached patch provides a relationship to items that contain references to the current item.

drewish’s picture

subscribing

netw3rker’s picture

I agree with Daren Oh. There is definitely a need for this functionality in cck/views2. Yched suggested that this be fixed by allowing the 'node' display to follow relationships, which is a good idea, however, sometimes a user will simply want to "show all nodes that have a parent of x". In this case, this is simply done by doing a node view, then adding the new "node_reference_referrer" field as a relationship (thanks daren for your patch). Then simply selecting node->nid argument and use a relationship.

In addition to the problem above, I also had the need to allow users to select the parent from an exposed filter (dropdown). The patch daren submitted doesnt have any filter handling built in, so i've included that in. the filter also lets users select the parent type that they want to show (or none for all). this way if the node_reference field is shared across multiple content types, it can be easilly restricted in the view.

Hope this helps!

scroogie’s picture

subscribe. nice functionality.

markus_petrux’s picture

I needed to dynamically build back reference views when working on the Node Relationships module, and I could do it using the relationship already exposed by noderef fields.

So, I don't see if another relationship is really needed. Can anyone describe when such a relationship is needed, or why the current relationship cannot be used to get the result?

Darren Oh’s picture

I'm not sure a what #31 was about, but some of us need to be able to include fields and use filters from referring nodes in our views, but only if referring nodes exist. I believe your solution requires referring nodes to exist.

markus_petrux’s picture

In views, when you have a node based view, you can add a relationship which can be required or not. Then when you add fields, filters, etc. you can choose if those fieds belong to the main table or the one provided by the relationship. This means you have two tables joined with INNER JOIN (required relationship) or LEFT JOIN (non-required relationship). And I think this can be used to resolve any situation.

I may miss something. Could you please post a query that is not possible with current relationships?

RIGHT JOIN makes no sense, IMHO, because if you have a noderef pointing to a non-existing node, that's an orphan.

Darren Oh’s picture

Why not?

SELECT node.nid AS nid,
   node_data_field_song.field_song_nid AS node_data_field_song_field_song_nid,
   node.type AS node_type,
   node.vid AS node_vid,
   node_og_ancestry.title AS node_og_ancestry_title,
   node_og_ancestry.nid AS node_og_ancestry_nid,
   node_ec_product_parcel.nid AS node_ec_product_parcel_nid,
   ec_product.price AS ec_product_price,
   ec_product.nid AS ec_product_nid,
   node_data_field_audio_preview.field_audio_preview_fid AS node_data_field_audio_preview_field_audio_preview_fid,
   node_data_field_audio_preview.field_audio_preview_list AS node_data_field_audio_preview_field_audio_preview_list,
   node_data_field_audio_preview.field_audio_preview_data AS node_data_field_audio_preview_field_audio_preview_data,
   node_node_data_field_song.nid AS node_node_data_field_song_nid,
   node_node_data_field_song.type AS node_node_data_field_song_type,
   node_node_data_field_song.vid AS node_node_data_field_song_vid,
   song_file.fpath AS song_file_fpath,
   node_data_field_weight.field_weight_value AS node_data_field_weight_field_weight_value
 FROM node node 
 LEFT JOIN content_type_song_file node_data_field_song ON node.vid = node_data_field_song.vid
 LEFT JOIN node node_node_data_field_song ON node_data_field_song.field_song_nid = node_node_data_field_song.nid
 LEFT JOIN og_ancestry node_node_data_field_song__og_ancestry ON node_node_data_field_song.nid = node_node_data_field_song__og_ancestry.nid
 LEFT JOIN node node_og_ancestry ON node_node_data_field_song__og_ancestry.group_nid = node_og_ancestry.nid
 LEFT JOIN ec_product_parcel ec_product_parcel ON node.nid = ec_product_parcel.mnid
 LEFT JOIN node node_ec_product_parcel ON ec_product_parcel.vid = node_ec_product_parcel.vid
 INNER JOIN term_node term_node_value_0 ON node_node_data_field_song.vid = term_node_value_0.vid AND term_node_value_0.tid = 16
 LEFT JOIN ec_product ec_product ON node.vid = ec_product.vid
 LEFT JOIN content_type_song node_data_field_audio_preview ON node_node_data_field_song.vid = node_data_field_audio_preview.vid
 LEFT JOIN song_file song_file ON node.vid = song_file.vid
 LEFT JOIN content_field_weight node_data_field_weight ON node.vid = node_data_field_weight.vid
 WHERE (node.status <> 0) AND (node.type in ('song_file')) AND (node_node_data_field_song.type in ('song')) AND (term_node_value_0.tid = 16)
   ORDER BY node_data_field_weight_field_weight_value ASC
markus_petrux’s picture

Sorry, but I still don't get it. Could you please describe what in this query cannot be done using currently implemented relationships?

For each relationship added to a view, you can choose which table already in the query it should join, and you can also choose if the relationship is required (INNER JOIN or LEFT JOIN). Then, for each field, filter, etc. you can choose to which relationship is related or just related to the main table of the view.

Could you please post a simple example (for dummies, I think this is my day today) of a query that cannot be done in views? What I don't get is the operation you're trying to do. And I think anything like this is already possible in Views 2.

As far as I can see, your query above uses LEFT JOINs, and this can be done using non-required relationships in Views 2.

Aside, when you're looking for Back references, you want INNER JOINs because the noderefs fields should refer to existing nodes, otherwise you have orphans that should be resolved in some other way (using phpMyAdmin, or this little thing, for example).

Darren Oh’s picture

Create two node types: a plain type and a type with a nodereference field. Create two plain nodes and a node that refers to one of them. Create a view that displays both plain nodes, with the plain node titles as one field and the referring node title as another.

The result should be a table like this:

Node      Referrer
Title 1   Title 3
Title 2
markus_petrux’s picture

Ok, here's how I would do it:

- Let's use page and story, with field_test_noderef in story where referenceable types is page.
- Create 2 page nodes: page 1 and page 2.
- Create 1 story: story 1, where noderef field references page 2.
- Create a view based on nodes.
- Add a relationship by the noderef field in story (non-required).
- Add a field: title (do not use relationship), label "Page node".
- Add a field: title (attached to noderef relationship), label "Story referrer".
- Add a filter by node type (do not use relationship), set to "Is one of Story".
- Enable the pager in case you already have more content.

Result is:

Page node    Story referrer
---------    -----------------
Page 1
Page 2       Story 1

If the relationship is required, then result is:

Page node    Story referrer
---------    -----------------
Page 2       Story 1

This is using latest dev snapshot of Views 2. Maybe that makes a difference.

markus_petrux’s picture

Here's the view I used.

Darren Oh’s picture

That requires us to filter by Story type. What if we had story1 and story2 types and needed a list of page nodes with fields for story1 and story2 referrers? The reverse would simplify things greatly in that case.

I'm trying to simplify the cases I have come across in real life.

markus_petrux’s picture

Status: Needs review » Postponed (maintainer needs more info)

Well, I think you can resolve any situation adding relationships are required and attaching them to the proper previous relationships. Then same with fields, filters, etc.

Please, provide an example as simple as possible where something cannot be done with currently implemented relationships, or where there is performance gain doing it with an additional relationship as in #29.

I'm very sorry, but I don't see the need yet, and if I have to commit this, I would like to be completely sure we add something for a reason. I could also look at another direction, but I decided to chime in here, and that tries to be constructive from the point of view of CCK. Again, I'm sorry, but I don't see the need. It might just be me.

drewish’s picture

I'll give it another look with the latest version of CCK and Views but what I was trying to do when I'd subscribed to this was using node references to put audio nodes into playlists and then build RSS feeds of them. So I'd want an argument for the playlist node but then get an RSS feed display of all the audio nodes. At that point there was no way to get the RSS display to list the audio nodes it only wanted to list the playlists they were attached to.

markus_petrux’s picture

Status: Needs review » Needs work

Ok, this is a brand new day here, I was able to sleep a little, and now I think I got it.

My view in #39 can be used to list all stories (referrer entities) and show if they have a referenced page. What I cannot do is list all pages and show if they have a story that refers to them.

So I tried with the patch in #29, and it seems it can do it. But there are some problems here:

Problem 1: I'm getting a join like this:

LEFT JOIN drupal_node node_referrer_node_data_field_test_noderef ON referrer_node_data_field_test_noderef.vid = node_referrer_node_data_field_test_noderef.nid

Note the condition compares vid against nid.

I copy/pasted the query from Views UI to phpMyAdmin just to fix that and keep playing.

Problem 2: If my story has revisions, I get duplicated pages. I'm not sure how to prevent this. How could I list only the active revision?

Problem 3: 'title short' needs to be check_plain'd here.

Problem 4: The relationship implemented by the patch in #29 generates an additional relationship that looks like this:

Content: Test Noderef referrer (field_test_noderef) - vid
Node reference referrer - Appears in: Story

However, currently implemented relationships look like this:

Content: Test Noderef (field_test_noderef)
Node reference - Appears in: Story

Which is confusing.

I'm wondering if it would be possible to just have one relationship definition that can be configured to be one way or the other. That way the difference would be more explicit as one would have to select the way in the relationship settings. If that's not possible or it may break existing views, then I think the new relationships need a title that matches the format, but it also needs to make explicit the difference between both (adding just "referrer" is confusing, I think; not sure about the word "vid" here).

In addition to this, if someone could write a handbook page about noderef relationships that would be really great.

@Darren Oh: Thanks for your patience. It took me a while to figure out, sorry.

[EDIT]
Typo in problem 4: I said #39 but I really meant #29. Fixed.

markus_petrux’s picture

Status: Postponed (maintainer needs more info) » Needs work

I've been thinking a bit more about it, and I believe this is a bit more complex...

When you want a back reference relationship to a node, this may be related to any nodereference field that is able to generate these references, so ideally, such a relationship would have to be configurable, and it should ask you for the node reference field that you want to use for this particular back reference relationship. Otherwise, we may end up with a bunch of these, as each node reference field would have to expose one for each node type.

I mean it should be something on the line of date_api_filter_handler. That filter is generic, and it asks you for the date field you want to use as an option in the filter settings form in views. Well, so these back references would have to be implemented in a similar fashion. Otherwise, we will be adding more clutter to the views ui (usability), and it will much harder to understand when you need one of these or not.

Darren Oh’s picture

It sounds like you're looking at the patch in #31 instead of the one in #29. Adding a special filter is really a separate issue that can be dealt with after we have a relationship to work with.

Darren Oh’s picture

Status: Needs work » Needs review
FileSize
4.37 KB

Resubmitting patch to avoid confusion in the future. Slightly cleaner, too.

markus_petrux’s picture

Actually no, I was testing your patch in #29. My comments in #44 and #45 still stand.

markus_petrux’s picture

1) 'base field' => 'vid' is required for the reverse node reference relationship. Otherwise, the join between the field table and the node table ends up comparing vid = nid. I have not tested the reverse user reference relationship, so I can't tell here.

2) Following the example view in #39 + #40, if stories have revisions AND the relationship is not required (LEFT JOIN), then we get duplicated records for each story, even for story revisions that do no refer to the corresponding pages. I consider this a bug that should be solved. The only way to solve this is to provide a views relationship handler that inserts additional query conditions to prevent this. Example:

SELECT node.nid AS nid,
   node.title AS node_title,
   node.uid AS node_uid,
   node.type AS node_type,
   node_revisions.format AS node_revisions_format,
   node_node_data_field_test_noderef_backref.title AS node_node_data_field_test_noderef_backref_title,
   node_node_data_field_test_noderef_backref.nid AS node_node_data_field_test_noderef_backref_nid,
   node_node_data_field_test_noderef_backref.uid AS node_node_data_field_test_noderef_backref_uid,
   node_node_data_field_test_noderef_backref.type AS node_node_data_field_test_noderef_backref_type,
   node_node_data_field_test_noderef_backref__node_revisions.format AS node_node_data_field_test_noderef_backref__node_revisions_format
 FROM drupal_node node 
 LEFT JOIN drupal_content_type_story node_data_field_test_noderef_backref ON node.nid = node_data_field_test_noderef_backref.field_test_noderef_nid
 LEFT JOIN drupal_node node_node_data_field_test_noderef_backref ON node_data_field_test_noderef_backref.vid = node_node_data_field_test_noderef_backref.vid
 LEFT JOIN drupal_node_revisions node_revisions ON node.vid = node_revisions.vid
 LEFT JOIN drupal_node_revisions node_node_data_field_test_noderef_backref__node_revisions ON node_node_data_field_test_noderef_backref.vid = node_node_data_field_test_noderef_backref__node_revisions.vid
 WHERE node.type in ('page')

   AND (node_data_field_test_noderef_backref.vid IS NULL OR node_node_data_field_test_noderef_backref.vid IS NOT NULL)

The last condition in the query is added by me in phpMyAdmin to get the desired result. It should be easy to see what I mean. All you needs is:

- content types page and story, then add a noderef field named 'field_test_noderef' to the story.
- Create a couple of pages, create one story and one revision for this story.
- Then run the above query with and without the last condition.
- Try this is views. You cannot create the last condition in views, at least I haven't found how. We need a relationship handler that does this for us I think.

3) Since we potentially need a new views relationship handler, then we may not need to expose all these relationships to views, but just one that asks the user which reference field is to be attached to the relationship. This is what I was telling at #45. This might not be an strict requirement, but I think it would be nice as this method would not add more clutter to the views ui. Date API exposes filters and arguments this way. Try adding a Date filter to see what I mean. The filter options ask you which data field you wish to attach with that particular filter to the view. So, I mean the same thing, but just for these reverse node/user references.

Summary: Fixing 1) and 2) are a must. 2) requires a specialized views relationship handler that is able to add conditions to the query to filter non-related revisions. When 2 is done, I think 3) is not so complex, and has the potential to not add more clutter to the views ui.

If 3) is not included, then I would replace the word "referrer" by "back reference" for the relationships titles, so these are named as existing noderef relationships, but suffixed with "back reference". This makes it easy to find them and figure out the difference in the views ui.

Darren Oh’s picture

Status: Needs work » Needs review
FileSize
4.4 KB

1) Fixed in attached patch. uid is the primary key for users, so it's not an issue there.
2) There is a Distinct option in the basic settings for situations like this.

markus_petrux’s picture

Status: Needs review » Needs work

Have you tried the distinct option?

Using the example query I posted above, I get this:

SELECT DISTINCT(node.nid) AS nid,
   node.title AS node_title,
   node.uid AS node_uid,
   node.type AS node_type,
   node_revisions.format AS node_revisions_format,
   node_node_data_field_test_noderef_backref.title AS node_node_data_field_test_noderef_backref_title,
   node_node_data_field_test_noderef_backref.nid AS node_node_data_field_test_noderef_backref_nid,
   node_node_data_field_test_noderef_backref.uid AS node_node_data_field_test_noderef_backref_uid,
   node_node_data_field_test_noderef_backref.type AS node_node_data_field_test_noderef_backref_type,
   node_node_data_field_test_noderef_backref__node_revisions.format AS node_node_data_field_test_noderef_backref__node_revisions_format
 FROM drupal_node node 
 LEFT JOIN drupal_content_type_story node_data_field_test_noderef_backref ON node.nid = node_data_field_test_noderef_backref.field_test_noderef_nid
 LEFT JOIN drupal_node node_node_data_field_test_noderef_backref ON node_data_field_test_noderef_backref.vid = node_node_data_field_test_noderef_backref.vid
 LEFT JOIN drupal_node_revisions node_revisions ON node.vid = node_revisions.vid
 LEFT JOIN drupal_node_revisions node_node_data_field_test_noderef_backref__node_revisions ON node_node_data_field_test_noderef_backref.vid = node_node_data_field_test_noderef_backref__node_revisions.vid
 WHERE node.type in ('page')
 GROUP BY nid

And I loose the row that was showing the story title while the one that doesn't have title because the node revision does not refer to the page is shown.

Any other idea?

markus_petrux’s picture

I think I've found a method to build the join to get what such a relationship needs. This is using phpMyAdmin. Please, note how the JOIN with the noderef field is built.

SELECT node.nid AS nid,
   node.vid AS vid,
   node.title AS node_title,
   node.type AS node_type,
   node_data_field_test_noderef_backref.nid AS field_nid,
   node_data_field_test_noderef_backref.vid AS field_vid,
   node_data_field_test_noderef_backref.field_test_noderef_nid,
   node_node_data_field_test_noderef_backref.title AS backref_title,
   node_node_data_field_test_noderef_backref.nid AS backref_nid,
   node_node_data_field_test_noderef_backref.vid AS backref_vid,
   node_node_data_field_test_noderef_backref.type AS backref_type
 FROM drupal_node node 

 -- Use INNER JOIN when relationship is required.
 LEFT JOIN (
   drupal_content_type_story node_data_field_test_noderef_backref
     INNER JOIN drupal_node node_node_data_field_test_noderef_backref
       ON node_data_field_test_noderef_backref.vid = node_node_data_field_test_noderef_backref.vid
 ) ON node.nid = node_data_field_test_noderef_backref.field_test_noderef_nid

 WHERE (node.type in ('page'))

This seem to work ok with node revisions. Otherwise, when the table that contains the noderef field has revisions, we end up with a lot of duplicate records.

I guess this could be done in Views creating a relationship handler that uses a custom views join class that writes the INNER JOIN with the related node table inside parenthesis.

markus_petrux’s picture

Writing a nested join as in #52 is quite complex. I've been spending some nights investigating that, and it seems to me I'm getting closer. For the moment, it seems to me we need a new relationships handler, and here we need to implement a new Views join_handler to alter the way the 2 joins that are added to the query are built.

Hints on how to do this in views are much appreciated.

markus_petrux’s picture

Status: Needs work » Needs review
FileSize
8.83 KB

Well, it's been tricky, but I think I got it.

Attached patch provides reverse relationships for node reference fields. Here's the Doxygen for the new relationships handler:

/**
 * @file
 * Handles reverse references using a nested join.
 *
 * Tipical relationships for node reference fields generate two joins that
 * look like this:
 *
 * @code
 *   FROM {node} node
 *     LEFT JOIN {content_field_table} node_data_field_table ON node.vid = node_data_field_table.vid
 *     {join-type} JOIN {node} node_node_data_field_table ON node_data_field_table.field_nid = node_node_data_field_table.nid
 * @code
 *
 * However, this kind of joins handle relationships between referrer nodes
 * and their parents. A referrer node can display information about the
 * referred node, but not the other way around. We cannot display fields
 * of the referring node from a parent node because the link goes from child
 * to parent.
 * To provide a relationships from parent to child, we need to generate a join
 * from the nid of the parent node to the reference field on the referrer node,
 * and because this is using the nid, we should do it in a way that does not
 * generate duplicate records when the referrer nodes have support for node
 * revisions enabled.
 *
 * This can be done using a nested join like the following:
 *
 * @code
 *   FROM {node} node
 *     {join-type} JOIN (
 *       {content_field_table} node_data_field_table
 *       INNER JOIN {node} node_node_data_field_table ON node_data_field_table.vid = node_node_data_field_table.vid
 *     ) ON node.nid = node_data_field_table.field_nid
 * @code
 *
 * Note that {join-type} depends on whether the relationship is
 * required (INNER) or not (LEFT).
 */

I believe reverse relationships for user reference fields could be provided on a follow up patch. I'm not sure what that should really do, yet, but I feel what we need is a different method. It seems to me we need to create a join between the user table and the user reference field in a similar way it is done in Content Profile. That's a different method, so maybe it is better to concentrate in node references for now.

alippai’s picture

I have 2 issues with the patch (comment #54):

  • There is no delta option on reverse reference - like at multiple nodereference fields
    1. Create an article -> gallery <- image reference structure
    2. Create a view of articles
    3. Add article -> gallery relationship as without the patch
    4. Add the gallery <- image relationship (which comes with the patch) - select the article->gallery reference as relationship (which was addad in the 3. step)
    5. You'll get an error about an unknown column

Thanks for your work, tomorrow I'll check how does this relationship work and help with it.

markus_petrux’s picture

@alippai: Thanks for testing. I think I've found the problem.

With the patch in #54 applied, apply the following to modules/nodereference/views/handlers/nodereference_handler_relationship_reverse.inc:

         $main_join->construct();
+        $main_join->adjusted = TRUE;
         $this->table_alias = $this->query->ensure_table($this->table, $this->relationship, $main_join);

Here's an updated patch that includes this fix. And now you should be able to add any number of reverse relationships stacked to other relationships previously defined. I tried with 3 and seemed to work ok here.

markus_petrux’s picture

Status: Needs review » Needs work

Not yet there.

I just noticed that node tables joined are always the main node table of the query and not the one provided by the relationship a new relationship is tied to.

I will investigate this...

markus_petrux’s picture

Status: Needs work » Needs review
FileSize
8.94 KB

I think the following fixes the problem described in #57.

         $main_join_definition = $main_join->definition;
+        $main_join_definition['left_table'] = $this->relationship;
         $main_join_definition['left_field'] = $this->definition['left_field'];

Patch updated.

BTW -> Re: "There is no delta option on reverse reference - like at multiple nodereference fields".

I would prefer to concentrate in working out the main challenge, which is find a way to implement reverse relationships, and then we can think about adding delta options for multiple value fields.

markus_petrux’s picture

Re: "There is no delta option on reverse reference - like at multiple nodereference fields".

Ok, now there is.

alippai’s picture

It works for me over multiple relationships, required/non-required, I can select fields, filefield etc.

"There is no delta option on reverse reference - like at multiple nodereference fields" - we were not talking about the same things.

You made the change for nodes with multiple value nodereference field defined, this is what I was talking about:
You have galleries and images as node types, one image has one nodereference field, which defines the parent node. You have 20 images, each with one nodereference field and you want to list your galleries with one of their child images.

May be my logic is wrong, but we need subquery (subview???), not joins to achieve this, this is why I wrote that note in comment #55. I know this is not as effective as joins, but can we attach this feature request to the roadmap of this issue? (or is there a more simple solution?)

markus_petrux’s picture

Ah, I see what you mean. Well, I think subqueries is out of the scope of this feature. Maybe you can solve this by using a custom views field that prints an image per gallery. See: http://drupal.org/project/views_customfield

[EDIT] Another possible approach for your use-case: Maybe you can filter images somehow, so that only one per gallery is selected.

Other than that, thanks for testing. Much appreciated. :)

@all: Before this is committed, I would appreciate more feedback. While it may work, I'm not sure the way I coded the views handler is the best way to do it. :-|

markus_petrux’s picture

Title: Reverse node-reference views argument with appropriate index management » Reverse node-reference views relationship

Better title. The indexing issue was resolved by #231453: Allow indexing columns.

miro_dietiker’s picture

Omg.. i was spending a lot of time working on "the same old story"... There are so many issues around seems like i didn't find the right though... Here we go!

I've done an own module right ready to publish to implement reverse relationship, because i've read somewhere reverse relationship won't get into cck.

I'm stopping publication (i was right in adding a nodereference_reverse.module project page) till we have a common point and i really hope it is not needed anymore.

May you please take the time and have a look at how i implemented the views handler. I'm very interested in not going an own way. My contributor request for nodereference_reverse:
http://drupal.org/node/552944
I've also explained the issue in nodereferrer.module with vid matching needed.
http://drupal.org/node/545396

Please tell me what i may do to support you bringing the cck integrated solution to the final release.

markus_petrux’s picture

@miro_dietiker: The problem I see with your approach is that, when relationship is not required, the view generates duplicate records when node revisions are used by the referrer node. This is something I solved here using nested join, which is not directly supported by Views, AFAICT, so the handler I implemented here is kind of tricky, but I think it works and does not generate dups.

If we can finally validate this issue, then reverse nodereferences would be provided by CCK itself. I haven't heard this was not going to get in, so I assume we can. :)

moshe weitzman’s picture

@markus - would be great if you could look over the fields api implementation in core and assure that some of your cool 6--3 work is still possible in d7.

markus_petrux’s picture

@moshe: I'm in the last stage of a project that has been taking a year or so to migrate a high traffic site from a proprietary CMS to Drupal, and that's D6. I would love to get more time to look at D7 in deep, but I won't be able, at least, until the end of September or so. I think we talked about this privately already.

PS: BTW, nice offtopic. lol

miro_dietiker’s picture

@markus_petrux:
I completely agree with your point. The solution you explain generally is perfect. IBTW: didn't test till now, i just read code.

What do you expect happens when node permissions node_access occur? At least we need to have a well defined behaviour.

I completely agree with your interpretation of query() in that all must be brought in at ensure_my_table().
Most of all i like your idea of join nesting by adding the params. Conditionally blanking it out in join() is a little hackish, but how else... The limitation to one join only is pretty OK for this solution: it's a nodereference nested join only. Modifying the join with preg_replace is still very interesting and minimalistic. ;-)

Most of the complexity i see is in nodereference_nested_join. So it would be pretty important to explain why you do it and what happens in code for others to understand and further maintenance.

However one very important issue:
http://bugs.mysql.com/bug.php?id=1591
Subqueries are only supported starting from mysql 5.1. Even debian lenny does only bring 5.0.51a ... and due to my (limited) test it even works... hmm.
Generally this is a pretty challenging advanced dependency. The only thing available before would be a subselect while i'd expect this would lead to major performance issues.

What do you think?

markus_petrux’s picture

hmm... I'm starting to think this feature cannot be included in CCK. Not even for Fields in D7, since Drupal 7 is aimed to support MySQL 5.0.x. :(

Where we are now?

1) Latest patch from Darren Oh at comment #50. It uses existing content_handler_relationship class.
2) Using nested joins as in latest patch at comment #59.
3) Adding a join to node table as in miro_dietiker's nodereference_reverse module as posted at #552944: miro_dietiker [mirodietiker]

Pros and Cons:

2) Due to compatibility issues with current MySQL requirements, that's a no, no. I think.

1) and 3) offer pretty similar results. Pros: this approach is compatible with existing MySQL requirements. Cons: Generates duplicate records when referring nodes support revisions.

So... maybe this would be better implemented as a contrib module? This module could perfectly be miro_dietiker's nodereference_reverse, where I would suggest using nested joins whenever it is possible, maybe choosing a different relationship handler depending on DB server installed, or doing the db_version() logic in the handler itself, and downgrading to normal join for those that do not support nested joins. This module would still live in D7 because D7 supports MySQL 5.0, but according to MySQL bug #1591, nested joins do not work until 5.1.

And we would mark this issue here as "won't fix".

Thoughts?

miro_dietiker’s picture

Just to complete your list with short thoughts:
4) adding a new nodereference-field column to mark current vid entries as active in cck.nodereference.
5) Or even adding a helper table.

markus_petrux’s picture

Status: Needs review » Needs work

Ok, so let's mark this as "needs work" for the moment.

However, I think this cannot be committed to CCK repository unless it offers support for the same DB engines supported by Drupal itself, and it provides a method to deal with node revisions without causing duplicates, and without adding a significant performance impact. Hence, I'm tempted to say "won't fix" here and move the discussion to nodereference_reverse module queue, for example.

In regards to 5), I think this is what nodereferrer module does, but that means the relation data is duplicated, which is IMHO bad design. It could live in contrib, but not a valid solution for CCK itself.

KarenS’s picture

Yeah, if there is any issue about requiring a specific version of MYSQL it definitely has to be a separate contrib module. And we also must support other dbs, so that would have to be tested too. As a separate contrib module you could create something that only works on specific dbs or have alternatives depending on the dbs or whatever.

So I would make a separate project for this.

miro_dietiker’s picture

Nodereferrerr views integration is much more broken (has not even vid matching and always shows all revision duplicates... except content_access enables implicit DISTINCT...).
It introduces CCK widgets and internal caching for widget display in bidir ways.
See my originally referred issues if it is of interest.

Let me try to explain my intention
If we add a column called "current" and only the most recent vid of a node contains 1 while all obsolete contain 0.

FROM {node} node
{join-type} JOIN {content_field_table} node_data_field_table
 ON node.nid = node_data_field_table.field_nid
INNER JOIN {node} node_node_data_field_table
ON node_data_field_table.vid = node_node_data_field_table.vid

to

FROM {node} node
{join-type} JOIN {content_field_table} node_data_field_table
 ON node.nid = node_data_field_table.field_nid
 AND node_data_field_table.current=1
LEFT JOIN {node} node_node_data_field_table
ON node_data_field_table.vid = node_node_data_field_table.vid

LEFT join is needed not to reduce queries and is OK since the field is a composite of the node.
This only needs in case of save to invalidate current=0 for previous vid and set current=1 for the active revision.

No special version required, no subselect, no nested joins, ...
But would it be a candidate for CCK integration?

BTW: Still my concerns about right content_access application are present. Does someone know more?

markus_petrux’s picture

Status: Needs work » Closed (won't fix)

Re: "But would it be a candidate for CCK integration?"

I'm afraid the answer is no. There should be a way to resolve this using SQL without adding clutter to the database, then try to find its way into views.

Re: "Still my concerns about right content_access application are present"

content_access() is invoked by views in different places. First, as an access callback of the fields, which happens before the query is executed to filter out fields where access is denied. Second, content_access() is invoked indirectly by content_format() when rendering each field. So, I think content_access() should not affect joins. I'm not sure if views discards a join when it is not needed though.

Honestly, I think this needs to mature a bit more, and I think the best place is a separate project where different implementations could be provided based on existing server resources. And maybe after some time, it is mature enough to be included in CCK.

I'm going to mark this as "won't fix" and let's move the discussion to nodereference_reverse module queue.

I'm going to stick with custom module using nested join for the project I'm working on. Miro, if you could include this method or anything equivalent, then I would switch to your module.

This issue can be re-opened when there's a clean way to implement this feature with full support to Drupal DB requirements, with support for node revisions, and not adding significant performance impact.

markus_petrux’s picture

@miro_dietiker: Please, let us know when/if you end up publishing your nodereference_reverse module. ...or have you abandoned that idea?

I'm currently using a custom module for the project I'm working on, which is based on the "nested join" approach I posted here. And maybe that could be published as a contrib, so others can benefit from that when their DB engine supports nested joins?

miro_dietiker’s picture

@markus_petrux: I will publish nodereference_reverse as you suggested due to the won't fix state.
And i see it also could make sense to integrate the different solutions..

However this needs some time to refactor. At least what i want is persistent configuration / naming before publishing it. So there's no hassle with upgrade pathes. I'll change my identifiers to your suggestion.

I'm looking forward to do the work within the next week.

markus_petrux’s picture

Good to know. Please, let me know if you need my custom solution (it is based on the latest patch I posted here though), as we've been using it for a while, and it works like a charm. :)

donquixote’s picture

Any news on the nodereference_reverse front?

Darren Oh’s picture

NodeReferrer attempts to address this need.

markus_petrux’s picture

Last time I checked NodeReferrer was prone to inconsistencies because it was duplicating relation data.

@miro_dietiker: It looks like you're not going to publish that module, right? If so, then I think I could publish the one we're using (or I can send it to you, if you wish to create the project), so it could be an option for those that run newer versions of MySQL, at least.

gg4’s picture

Look forward to this being published. Thanks.

markus_petrux’s picture

Soon to be committed as a separate contrib:

http://drupal.org/project/reverse_node_reference

@miro_dietiker: I can transfer the project to you whenever you wish, or I can add you as co-maintainer, if you prefer.

miro_dietiker’s picture

markus, i'd like to contribute..
i wasn't able to focus on this topic and push the project but i'd like to participate and improve the situation.

mstrelan’s picture

subscribe

markus_petrux’s picture

@miro_dietiker: I sent you an email a few weeks ago, did you got it?

nally’s picture

Can anyone explain how this work is related to the http://drupal.org/project/backreference module?

It seems to me, that the BackReference module is a clean solution to this requirement.

Is it that folks don't want to double up on their number of fields?

Any insight is appreciated.

nally’s picture

Ah... just read this, and it seems to answer the question, but for a different module.

http://drupal.org/node/798786

Does anyone have anecdotal evidence that using something like BackReference is a bad idea for that type of relationship?

markus_petrux’s picture

@miro_dietiker: Please, see #84. I'm using this channel because it has been impossible to contact you privately, and I think the resolution on this CCK request is of community interest. I wanted to talk with you about the new module, I gave you dev access, but I have just removed it now, because I think it is time to create a stable release of the Reverse Node Reference module, and I want to make sure no critical changes are committed (IIRC, your approach in coding this module was pretty different). Please, let me know if you're still interested to co-maintain that module. I would be happy to even transfer the mainteinership, but I would like to make sure we actually have a solution that works ok with node revisions and multiple value fields. Cheers

doublejosh’s picture