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.

Files: 
CommentFileSizeAuthor
#12 648516-node-admin-taxonomy-revision.patch540 byteswilliamestrada
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#11 648516-node-admin-taxonomy-entry-duplication.patch858 bytescafuego
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#7 648516.patch685 bytescafuego
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 648516_2.patch.
[ View ]
#6 648516.patch685 bytescafuego
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 648516_1.patch.
[ View ]
#5 admin-content-node-pager-D6.patch685 bytescafuego
#3 648516.patch745 bytescafuego
Passed on all environments.
[ View ]
#1 648516.patch745 bytescafuego
Passed on all environments.
[ View ]
admin-content-node-pager.patch747 bytescafuego
Unable to apply patch admin-content-node-pager.patch
[ View ]

Comments

Version:6.x-dev» 7.x-dev
StatusFileSize
new745 bytes
Passed on all environments.
[ View ]

Also a potential issue in D7, patch attached.

Status:Active» Needs review

StatusFileSize
new745 bytes
Passed on all environments.
[ View ]

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

Status:Needs review» Closed (fixed)

Not an issue with this query in D7.

Version:7.x-dev» 6.x-dev
Status:Closed (fixed)» Needs review
StatusFileSize
new685 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.

StatusFileSize
new685 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 648516_1.patch.
[ View ]

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

StatusFileSize
new685 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 648516_2.patch.
[ View ]

Ok, retry the test :-P

Status:Needs review» Needs work

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

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.

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.

Status:Needs work» Needs review
StatusFileSize
new858 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Attached an updated patch.

StatusFileSize
new540 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

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

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.

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

Can we get some more reviews on this?

hello?

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?

Priority:Normal» Major

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

Status:Reviewed & tested by the community» Fixed

Looks simple. Committed, pushed, thanks.

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

Status:Fixed» Closed (fixed)

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