[drupal-devel] [feature] Sort logs by ID instead of timestamp

[lgarfiel]

Issue status update for 
http://drupal.org/node/23560
Post a follow up: 
http://drupal.org/project/comments/add/23560

 Project:      Drupal
 Version:      cvs
 Component:    watchdog.module
 Category:     feature requests
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  njivy
 Updated by:   lgarfiel
 Status:       patch (ready to be committed)

Current policy or no, isn't part of the point of unique IDs in SQL that
you are guaranteed that they are unique, not that they are sequential? 
Relying on them to be sequential is a problem, because you can't always
guarantee that they will be.  


If you want greater than one second resolution for timestamps, I'd
suggest replacing the SQL timestamp filed with a microsecond-sensitive
numeric field and storing the timestamp that way.  It can then be
converted back to a YYYY-MM-DD (or something else) timestamp on the
fly, after it's already been sorted by the SQL query.  PHP's
microtime() [1] function would be useful here.


That should be database-agnostic, too.
[1] http://www.php.net/manual/en/function.microtime.php




lgarfiel



Previous comments:
------------------------------------------------------------------------

Wed, 25 May 2005 01:59:52 +0000 : njivy

In watchdog.module I propose changing the key used to sort logs
chronologically.  Instead of using the timestamp, which is accurate
only to the nearest second, we could use the log entry id ("wid" in the
database).  As I understand it, the IDs are assigned sequentially, so
this is a good approximation to the correct order in which the events
are recorded.  Also, I don't know of any events that are recorded after
a delay.


The problem with the current sorting key arises when logged events come
within 1 second of each other.  The order of events is not always
preserved when viewing the log because the timestamp is identical for
several events.


In "modules/watchdog.module" neard line 91, the word "timestamp" is
replaced with "wid":
$header = array(
    ' ',
    array('data' => t('Type'), 'field' => 'w.type'),
    array('data' => t('Date'), 'field' => 'w.wid', 'sort' =>
'desc'),
    array('data' => t('Message'), 'field' => 'w.message'),
    array('data' => t('User'), 'field' => 'u.name'),
    array('data' => t('Operations'), 'colspan' => '2')
  );


Preliminary testing suggests this is a good tweak.




------------------------------------------------------------------------

Wed, 25 May 2005 03:53:05 +0000 : Steven

Do auto_increment id's never go back to fill in deleted rows? We need to
make sure on pgsql and mysql.


Another option might be to sort by timestamp first, then by wid. I'm
not sure if the current tablesort supports that though.




------------------------------------------------------------------------

Thu, 26 May 2005 18:32:39 +0000 : Dries

Also, I think auto_increment IDs rotate/overflow.  This isn't very
likely though.  Currently, drupal.org's largest wid value is appr.
1,500,000.




------------------------------------------------------------------------

Thu, 04 Aug 2005 13:36:02 +0000 : killes at www.drop.org

Attachment: http://drupal.org/files/issues/wid.patch (782 bytes)

According to
http://dev.mysql.com/doc/mysql/en/example-auto-increment.html
auto_increments ususally do not get re-used.


The patch is attached and tested.




------------------------------------------------------------------------

Thu, 04 Aug 2005 13:44:07 +0000 : Jeremy at kerneltrap.org

Does the tablesort function allow using two keys to sort results?  If
you first sort on timestamp, then on wid, you wouldn't have to worry
about id's being reused or wrapping.




------------------------------------------------------------------------

Thu, 04 Aug 2005 14:18:02 +0000 : moshe weitzman

we made a decision a long time ago to support DBMS which don't have
autoincrement fields. As a result, we implemented our own sequences
table and almost all modules use this. Any module which relies on
autoincrement instead of sequences is buggy (according to current
policy).


if we change our policy on this, then we should rip out all the
sequences stuff. Until we do that, I can't support relying on
autoincrement. I'd love for others to chime in here - marking this as
'needs review'




------------------------------------------------------------------------

Thu, 04 Aug 2005 14:30:43 +0000 : killes at www.drop.org

@Jeremy: Sorting by two fields does not seem to work.


@Moshe: This code does not rely on the fact that wid is an
auto_increment field in any way. Just some concerns did.