2
\$\begingroup\$

I would like to test the following garbage collector code on a database of sessions. It should be following the conditions below:

  • if 'remember me' is enabled by the user, then it should delete all rows which are not active within 2 weeks.
  • if 'remember me' is disabled by the user, then it should delete all rows which are not active within 30 minutes.

I have times inserted in the LAST_ACTIVITY column, with the UNIX_TIMESTAMP() format.

public function gc( $maxlifetime )
{
    $this->conn->SQL( 'DELETE FROM session WHERE (LAST_ACTIVITY < ? AND REMEMBERUSER = ?) OR (LAST_ACTIVITY < ? AND REMEMBERUSER = ?)' , 
        [ strtotime('-30 minutes'), 'N' , strtotime('-2 weeks'), 'Y']);
    return true;
}

Are the AND or OR operators working properly? I would only like to test the SQL query, and not the gc method.

\$\endgroup\$
1
  • \$\begingroup\$ This probably belongs on StackOverflow. \$\endgroup\$ Feb 19, 2014 at 19:48

1 Answer 1

2
\$\begingroup\$

Observations

  • Your $maxlifetime parameter is unused.
  • One of the conditions is redundant: any session older than two weeks should be discarded unconditionally.
  • The capitalization of your column names is unconventional. Usually, identifiers are lowercase, and SQL keywords are ALL CAPS. (Note: identifiers in MySQL are case-sensitive on Unix, but not Windows.)

Proposal

DELETE FROM session
    WHERE last_activity < CURRENT_TIMESTAMP() - INTERVAL 2 WEEK
    OR (rememberuser = 'N' AND last_activity < CURRENT_TIMESTAMP() - INTERVAL 30 MINUTE);

Advantages

  • This is a fixed query with no parameters, which keeps things simple.
  • It's almost English-like in readability.

Caveats

  • This requires your last_activity column to be of the DATETIME or TIMESTAMP type. You could also make it work with integer Unix timestamps instead, using

    DELETE FROM session
        WHERE last_activity < UNIX_TIMESTAMP(CURRENT_TIMESTAMP() - INTERVAL 2 WEEK)
        OR (rememberuser = 'N' AND last_activity < UNIX_TIMESTAMP(CURRENT_TIMESTAMP() - INTERVAL 30 MINUTE));
    
  • This assumes that the last_activity times were set according to the database server's clock, not the application server's clock. Otherwise, if they are running on separate machines, or if you are careless with your treatment of timezones, you could end up with clock skew. Therefore, it's best to pick one clock and stick with it consistently. I prefer to use the database server's clock, for several reasons:

    1. If your application runs on multiple application servers, all connecting to a central database, then the database clock is the natural official time source.
    2. Your queries can be simpler, as illustrated above.
    3. Storing the timestamps in a DATETIME column is more meaningful than storing them as integers.

    So, to make this work, the last_activity times have to be inserted or updated using CURRENT_TIMESTAMP() or NOW(), or established by default when a TIMEZONE column has no specified value.

  • Admittedly, this formulation uses MySQL-specific date / time functions, so it is less portable. Similar solutions exist for other database systems, though.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.