Posted by bertboerland on August 4, 2005 at 8:47am
10 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | statistics.module |
| Category: | bug report |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | needs profiling |
Issue Summary
The order ionce clicked on "visitors" on the access logs part, is weird. first it sorts by username (0-9,a-z) which is good but once it gets to ip addresses, there is no logic and hence you cant find an ip address the easy way. try to lookup 11.12.13.14 in http://drupal.org/admin/logs/visitors?page=0&sort=asc&order=Visitor and you will see there is no logic there.
I thought ordening would be based on first octet, second octet, third and fourt (hence 1.2.3.4 would be before 2.255.255.255) but is not. I think there is some hash over the complete ip address and the ordening is done on that, because all other logic escapes me.
anyone cares to dig into this?
Comments
#1
#2
the query is getting both the username and the hostname (ip). all sorts are done on the username field only. as he prepares to display the results of the query, he swaps in the hostname if the username is empty. so the hostnames are never really sorted at all (and probably shouldn't be values in a sortable field :)
$rows[] = array($account->hits, ($account->uid ? theme('username', $account) : $account->hostname), format_interval(round($account->total / 1000)), $ban_link);
#3
This looks more like a feature request, and affects HEAD.
One solution to this problem would require to store IPs using a different format, maybe with the same approach implemented by phpBB. It uses CHAR(8) in the database, and converts the IP address to an hexadecimal value, where each part needs 2 bytes.
They use this functions:
<?phpfunction encode_ip($dotquad_ip) {
$ip_sep = explode('.', $dotquad_ip);
return sprintf('%02x%02x%02x%02x', $ip_sep[0], $ip_sep[1], $ip_sep[2], $ip_sep[3]);
}
function decode_ip($int_ip) {
$hexipbang = explode('.', chunk_split($int_ip, 2, '.'));
return hexdec($hexipbang[0]). '.' . hexdec($hexipbang[1]) . '.' . hexdec($hexipbang[2]) . '.' . hexdec($hexipbang[3]);
}
?>
That way IPs can be sorted on a more logical manner and is more optimal than converting IPs to longs.
#4
Forgot to mention side effects related to such a change...
1) Stuff related to {access_log} table (statistics module).
2) Stuff related to {access} table (ip bans, etc.).
3) Watchdog.
4) Probably some contrib modules too.
I don't believe this will be done any time soon... and I can't think of any other reasonable way to sort IPs logically.
#5
Did current versions taken this feature request in consideration?
Moving to current.
#6
#7
Needs to be fixed first in D8.
#8
WOW!! A seven-year-old issue... lets see if we cant just kill this one.
The attached patch fixes this "enough" IMO by always sorting the ip (ASC), but since this particular column in this particular table actually contains data from two DB columns (name & ip) it does not obey the tablesort link in the header.
I think this is an acceptable solution in this case though. If we want we can open up a feature request for orderByHeader() to accept multiple fields, but that is out of scope for this issue me thinks.
#9
Looks good to me, a very simple solution to the issue that has the possibility to get back-ported to D7 too.
Although, my preference would be to remove all of the access_log functionality from D8 and just keep the node_counter.
#10
There's no index on hostname, could we get a before/after EXPLAIN for the query with the default sort? 'needs profiling' isn't quite the right tag but in the interests of not creating a new tag, adding that.