Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.There's a code block like the following in the statistics_node_tracker function.
$result = pager_query('SELECT a.aid, a.timestamp, a.url, a.uid, u.name
FROM {accesslog} a LEFT JOIN {users} u ON a.uid = u.uid
WHERE a.path LIKE \'node/%d%%\'' . tablesort_sql($header),
30, 0, NULL, $node->nid);
It definitely seems wrong to me setting $node->nid in 'LIKE' parameter like above.
When you have nodes with nid 123 and another with 1234, you'll get access logs of both nodes if you query with $node->nid as 123.
I want to say another thing. This is just my opinion for improvement. Why don't you query access logs first for users. I don't think viewing all access logs by time is a good idea.
Instead query for users with access logs displaying his or her first and last access time. If you want to see all his or her logs, then there's a link to detailed logs. How about that? (Actually I am running my Drupal installation customized as such...) A query for that will be like this(for MySQL 5)
select u.uid, u.name, a.first_access, b.last_access
from {users} u
left join ( select uid, min(timestamp) first_access from {accesslog} where path like \'node/%d%%\' group by uid ) a on u.uid=a.uid
left join ( select uid, max(timestamp) last_access from {accesslog} where path like \'node/%d%%\' group by uid ) b on u.uid=b.uid
where u.uid > 0 order by a.first_access desc
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 160226-statistics-node-tracker-D6.patch | 1.08 KB | dave reid |
| #11 | 160226.fix_sql.patch | 1.17 KB | berdir |
| #8 | 160226-statistics-node-tracker-SQL-D7.patch | 5.47 KB | dave reid |
| #7 | 160226-statistics-node-tracker-SQL-D7.patch | 2.12 KB | dave reid |
| #3 | 160226.001.patch | 1.05 KB | karschsp |
Comments
Comment #1
dave reidMarked #363283: node/id/track wrong for a single digit id as a duplicate of this issue. Moving to 7.x to get fixed first.
Comment #2
dave reidConfirmed this is a bug.
I did a little search back through old versions of statistics.module (D5) and it appears that we have always searched by "LIKE 'node/%d%%'" when we only need to use "= 'node/%d'" since we use $_GET['q'] to insert into the accesslog.
Comment #3
karschsp commentedhere's a patch that should fix this.
Comment #4
mr.baileysAs Dave Reid suggested in #2: I don't think there's a reason to leave the "LIKE"-portion lingering if we're dropping the '%', so it should probably be replaced with '='.
Also, in this case I think we should replace the single-quotes by double-quotes so we can get rid of the ugly
\'node/%d\''Comment #5
damien tournoud commentedI believe the initial idea was to count every "node/xxx/yyy" where yyy can be /edit, /talk, /...
In that case, the correct query would be:
... but of course... we also need to convert this to DB:TNG and add a test ;)
Comment #6
dave reidActually I think testing for LIKE "node/x/" will not work because the node-view locations are just "node/x". We'll probably need to use a.path = "node/x" OR a.path LIKE "node/x/%"
Comment #7
dave reidPatch attached that coverts to new DBTNG pager/tablesort and fixes the SQL. Now I just need to get a quick test written.
Comment #8
dave reidFixed a couple other pager queries.
Comment #10
dries commentedThis patch will need to be re-rolled for Drupal 7. A statistics.module DBTNG patch just landed and conflicts with this one. Thanks!
Comment #11
berdirRe-roll.
Comment #12
berdirUpdate status.
Comment #13
dries commentedWhen looking at this patch, all of a sudden, I find myself wondering why we care about
'node/$nid/%'to begin with. Isn't'node/$nid'enough?Comment #14
dave reid@Dries: That's what I thought originally as well, but it appears to be intentional since the introduction of this code. As Damien said in #5, "I believe the initial idea was to count every "node/xxx/yyy" where yyy can be /edit, /talk, /..."
Comment #15
dries commentedCommitted to CVS HEAD. We can argue about the paths to include in a follow-up issue, if necessary.
Comment #16
dave reidBackported for Drupal 6.
Comment #17
gábor hojtsyThanks, committed.