On a Drupal site where all nodes default to 'create new revision' the {term_node} table may end up with a rather large amount of identical nid-tid entries, which all have a different vid.

When filtering nodes by category on admin/content/node, the automatically generated count query, which at that point uses an INNER JOIN on nid only and disregards vid,, will return nodes times vids results, whilst the actual select query only return node number of results.

This (on my Drupal) results in the pager displaying 2 pages for only 7 results. The first page contains two nodes, the second page contains five nodes.

By also using the vid in the JOIN in node_build_filter_query() this problem is resolved.

Patch attached.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cafuego’s picture

Version: 6.x-dev » 7.x-dev
FileSize
745 bytes

Also a potential issue in D7, patch attached.

cafuego’s picture

Status: Active » Needs review
cafuego’s picture

FileSize
745 bytes

Re-adding patch, so the test system can queue it :-)

cafuego’s picture

Status: Needs review » Closed (fixed)

Not an issue with this query in D7.

cafuego’s picture

Version: 7.x-dev » 6.x-dev
Status: Closed (fixed) » Needs review
FileSize
685 bytes

This issue seems to not apply to D7 anymore, as there appears to be no taxonomy filter in the content list anymore. However, the problem still exists in D6. I've modified the patch a bit, as there is no need to join on both nid and vid.

Updated patch attached.

cafuego’s picture

FileSize
685 bytes

Lets see if we can make the test bot test this patch.

cafuego’s picture

FileSize
685 bytes

Ok, retry the test :-P

Status: Needs review » Needs work

The last submitted patch, 648516.patch, failed testing.

tormu’s picture

I also noticed this bug today when I tried to list nodes that CURRENTLY have certain taxonomy term. Due to this bug, I also get nodes that did have the term in any of their revisions. So nodes that don't have the term now are listed, too.

jschrab’s picture

Wow, Drupal 6.22 and I'm coming across this problem for the first time. That is, for environments where revision keeping is turned on for a content type and taxonomy term choices are changing across revisions, THEN when searching for content by term names, ALL cases of that term choice across even old revisions are returned in the result set.

Good to hear this shouldn't be a D7 problem. But I'm seeing this in D6 today.

cafuego’s picture

Status: Needs work » Needs review
FileSize
858 bytes

Attached an updated patch.

webbywe’s picture

cafuego patch throws sql error -- "Unknown column 'v.nid' in 'on clause'". Fixed to be n.vid

cafuego’s picture

Status: Needs review » Reviewed & tested by the community

Indeed, it's what I had in my Drupals already ... must have forgotten to update the patch :-/

Working exactly as it should now, returning one unpaged list of nodes on my site, as opposed to a list of nodes with 9+ empty pages.

kattekrab’s picture

So where's this at now? Does the patch need to be resubmitted and retested?

Can we get some more reviews on this?

kattekrab’s picture

hello?

kattekrab’s picture

Would love to get a few more people to look at this - so we might have a chance of getting it committed some time soon?

kattekrab’s picture

Priority: Normal » Major

Just had another client "report" this bug to me. Can we get this committed please?

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Looks simple. Committed, pushed, thanks.

kattekrab’s picture

Yay! Thank you thank you so much @Gábor Hojtsy

Status: Fixed » Closed (fixed)

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