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.
Comment | File | Size | Author |
---|---|---|---|
#12 | 648516-node-admin-taxonomy-revision.patch | 540 bytes | webbywe |
#11 | 648516-node-admin-taxonomy-entry-duplication.patch | 858 bytes | cafuego |
#7 | 648516.patch | 685 bytes | cafuego |
#6 | 648516.patch | 685 bytes | cafuego |
#5 | admin-content-node-pager-D6.patch | 685 bytes | cafuego |
Comments
Comment #1
cafuego CreditAttribution: cafuego commentedAlso a potential issue in D7, patch attached.
Comment #2
cafuego CreditAttribution: cafuego commentedComment #3
cafuego CreditAttribution: cafuego commentedRe-adding patch, so the test system can queue it :-)
Comment #4
cafuego CreditAttribution: cafuego commentedNot an issue with this query in D7.
Comment #5
cafuego CreditAttribution: cafuego commentedThis 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.
Comment #6
cafuego CreditAttribution: cafuego commentedLets see if we can make the test bot test this patch.
Comment #7
cafuego CreditAttribution: cafuego commentedOk, retry the test :-P
Comment #9
tormu CreditAttribution: tormu commentedI 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.
Comment #10
jschrab CreditAttribution: jschrab commentedWow, 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.
Comment #11
cafuego CreditAttribution: cafuego commentedAttached an updated patch.
Comment #12
webbywe CreditAttribution: webbywe commentedcafuego patch throws sql error -- "Unknown column 'v.nid' in 'on clause'". Fixed to be n.vid
Comment #13
cafuego CreditAttribution: cafuego commentedIndeed, 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.
Comment #14
kattekrab CreditAttribution: kattekrab commentedSo where's this at now? Does the patch need to be resubmitted and retested?
Can we get some more reviews on this?
Comment #15
kattekrab CreditAttribution: kattekrab commentedhello?
Comment #16
kattekrab CreditAttribution: kattekrab commentedWould love to get a few more people to look at this - so we might have a chance of getting it committed some time soon?
Comment #17
kattekrab CreditAttribution: kattekrab commentedJust had another client "report" this bug to me. Can we get this committed please?
Comment #18
Gábor HojtsyLooks simple. Committed, pushed, thanks.
Comment #19
kattekrab CreditAttribution: kattekrab commentedYay! Thank you thank you so much @Gábor Hojtsy