turn subquery into join in the cron
deekayen - June 9, 2008 - 15:42
| Project: | Masquerade |
| Version: | 6.x-1.x-dev |
| Component: | Miscellaneous |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Wherever you want to put this, it cleans up the masquerade table when people don't use the switch back link.
/**
* Implementation of hook_cron()
*
* Cleanup masquerade records where people didn't use the switch back link
* that would have cleanly removed the user switch record.
*/
function masquerade_cron() {
db_query('DELETE FROM {masquerade} WHERE sid NOT IN (SELECT sid FROM {sessions})');
}
#1
#2
#3
This change will not work on MySQL 4.0, as it doesn't support sub-queries. Since Drupal 5 still supports MySQL 4.0, it would be good if this module did, too.
#4
#5
new query:
DELETE m FROM masquerade AS m LEFT JOIN sessions AS s ON m.sid = s.sid WHERE s.sid IS NULL;#6
Still doesn't work in MySQL 4.0 in 5.x-1.2. According to http://dev.mysql.com/doc/refman/4.1/en/delete.html, MySQL 4.0 and 4.1 use different syntax when table aliases are involved. It'd probably be better to not use aliases at all:
DELETE masquerade FROM masquerade LEFT JOIN sessions AS s ON masquerade.sid = s.sid WHERE s.sid IS NULL;
#7
Then the things that need to be bracketed look like this?
DELETE {masquerade} FROM {masquerade} LEFT JOIN {sessions} AS s ON {masquerade}.sid = s.sid WHERE s.sid IS NULL;#8
Nevermind, the problem I'm having is unrelated to this.
#9
It doesn't hurt anything to make it more compatible, I just didn't realize 4.0 was different than 4.1 on this. I'll commit the following shortly:
db_query('DELETE {masquerade} FROM {masquerade} LEFT JOIN {sessions} ON {masquerade}.sid = {sessions}.sid WHERE {sessions}.sid IS NULL');#10
OK. The problem I ran into after upgrading is described in #285472: Parse error with PHP 4.
Now that I've fixed it, I do get the following error when running cron, though:
user warning: Not unique table/alias: 'm' query: DELETE m FROM masquerade AS m LEFT JOIN sessions AS s ON m.sid = s.sid WHERE s.sid IS NULL in /var/home/team/volunteers/public_html/includes/database.mysql.inc on line 172.
Your latest suggested query seems to fix it.
#11
Automatically closed -- issue fixed for two weeks with no activity.
#12
Unfortunately the solution that was used here does not work
in PostgreSQL as the syntax for DELETE with joins is also
different than that of MySQL.
The best solution for portability is to use subqueries as was
suggested. It's true that Drupal 5 still supports MySQL 4.0
and lower which do not have subqueries but Drupal 6 requires
MySQL 4.1 which does support subqueries.
So any chance the query can be reverted to use subqueries
in the Drupal 6 version of masquerade so that PostgreSQL
users and MySQL users both run w/o error?
#13
Yep my PostgreSQL system is banging its head against this error every time Cron runs. Annoying. My fix is attached for 5.x 1.3.
#14
I committed #13 and reverted back to the original subquery version for #12 in 6.x.
#15
Automatically closed -- issue fixed for two weeks with no activity.
#16
Looks like I oops'ed on the 6.x version.
#17
Just missing a closing paren...
db_query('DELETE FROM {masquerade} WHERE sid NOT IN (SELECT s.sid FROM {sessions} AS s');should be
db_query('DELETE FROM {masquerade} WHERE sid NOT IN (SELECT s.sid FROM {sessions} AS s)');#18
Will be in Masquerade 6.x-1.1.
#19
#20
Automatically closed -- issue fixed for 2 weeks with no activity.
#21
Any chance of getting this fix in the Drupal 5 branch? I made the modification manually to my local copy of 5.x-1.3 and it works flawlessly on Postgres. It looks like the patch here will apply cleanly to 5.x-1.3.
#22
I'm staring at DRUPAL-5--1-3 and DRUPAL-5 and don't see a problem with the cron query. They're different than D6. What did you change?
#23
Original code was this:
function masquerade_cron() {// see http://drupal.org/node/268487 before modifying this query
db_query('DELETE {masquerade} FROM {masquerade} LEFT JOIN {sessions} ON {masquerade}.sid = {sessions}.sid WHERE {sessions}.sid IS NULL
');
}
which did not work with Postgres. I changed it to this:
function masquerade_cron() {// see http://drupal.org/node/268487 before modifying this query
if (in_array($GLOBALS['db_type'], array('mysql', 'mysqli'))) {
db_query('DELETE {masquerade} FROM {masquerade} LEFT JOIN {sessions} ON {masquerade}.sid = {sessions}.sid WHERE
{sessions}.sid IS NULL');
} elseif (in_array($GLOBALS['db_type'], array('pgsql'))) {
db_query('DELETE FROM {masquerade} WHERE {masquerade}.sid NOT IN ( SELECT {sessions}.sid FROM {sessions} )');
}
}
and it now works properly for me.
#24
Yeah, that's what was in the latest -dev copy. I turned it into a 5.x-1.4 release just now.
#25
Thanks, I'll give it a try!
#26
Shouldn't that query check the timestamp too instead of selecting all sid's from the sessions table? Like, you should only look back 24 hours before destroying the masquerade session, maybe less.
I am trying to pin down why PHP runs out of memory when cron runs and saw this hook.
**edit**
CAPTCHA employs this in its cron:
function captcha_cron() {// remove challenges older than 1 day
db_query('DELETE FROM {captcha_sessions} WHERE timestamp < %d', time() - 60*60*24);
}
Seems like this would make it a lot faster if you have thousands of sessions in the sessions table. Just a thought.
#27
gh0st25: no, because then someone who gets their masquerade deleted because they expect their session to last 3-4 days will file a bug about disappearing masquerade states. It probably would be faster, but if you're having performance problems related to the size of your sessions table, I recommend Session Expire instead of adding arbitrary time limits to the cleanup process here. Moreover, the subquery select for IN() is entirely database - it shouldn't have any impact on your PHP's instance of memory consumption.
#28
I forgot to mention that I've been running 5.x-1.5 with Postgres for a few months now with no issues.