Event module's use of HAVING is incompatible with db_rewrite_sql()

Thanks for reporting this. It's an Event bug — or a missing feature in D6 core.

Event's event_get_events_event() feeds a query like

SELECT n.nid 
  FROM node n 
  WHERE cond1 
  HAVING cond2
  ORDER BY event_start ASC

through db_rewrite_sql(), which returns

SELECT DISTINCT(n.nid) 
  FROM node n 
    INNER JOIN node_access na ON na.nid = n.nid 
  WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all'))) 
    AND (cond1 
      HAVING cond2)
  ORDER BY event_start ASC

Note how db_rewrite_sql() puts parentheses around cond1 HAVING cond2. That's illegal SQL.

This error will occur with any node access module, but I think at this point, ACL is the only one that has been ported yet.

I'm not sure why killes uses the non-standard HAVING here and won't try to provide a patch.

@killes: as you noted in http://drupal.org/node/151910#comment-681196, the patch that was committed in that issue removed support for HAVING (without a preceding GROUP BY), yet you continue to have queries of that type in
event_get_events_event()
event_get_events_user()
event_get_events_site()

http://dev.mysql.com/doc/refman/5.0/en/select.html doesn't make it completely clear whether HAVING without GROUP BY is allowed or not, but db_rewrite_sql() does not support it, and if any node access module is enabled, then Event fails.

See also #235248: Event module causes SQL errors!.

As this is an extremely urgent issue for me I'm going to try and get around it. I'll post my findings here... I doubt I can rewrite the queries correctly, sql isn't my strong point... It'll likely be some half-baked hack, but at least it'll be better than the errors we currently get...

Pobster

Okay I've fixed it, there actually is no 'GROUP BY' clause in any of the queries (at least not in event_database.mysqli.inc which is all I'm bothered about) so the sql is actually fairly simple to alter;

Line 32: $query = "SELECT n.nid, n.uid, n.title, n.type, e.event_start AS event_start_orig, e.event_end AS event_end_orig, e.timezone, e.has_time, e.has_end_date, tz.offset AS offset, tz.offset_dst AS offset_dst, tz.dst_region, tz.is_dst, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_start_utc, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_end_utc, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_user, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_user, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_site, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_site, tz.name as timezone_name FROM {node} n INNER JOIN {event} e ON n.nid = e.nid INNER JOIN {event_timezones} tz ON tz.timezone = e.timezone WHERE (event_start >= '%s' AND event_start <= '%s') OR (event_end >= '%s' AND event_end <= '%s') OR (event_start <= '%s' AND event_end >= '%s') && n.status = 1 AND ((e.event_start >= '%s' AND e.event_start <= '%s') OR (e.event_end >= '%s' AND e.event_end <= '%s') OR (e.event_start <= '%s' AND e.event_end >= '%s'))";

Line 61: $query = "SELECT n.nid, n.uid, n.title, n.type, e.event_start AS event_start_orig, e.event_end AS event_end_orig, e.timezone, e.has_time, e.has_end_date, tz.offset AS offset, tz.offset_dst AS offset_dst, tz.dst_region, tz.is_dst, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_start_utc, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_end_utc, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_user, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_user, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_site, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_site, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end, tz.name as timezone_name FROM {node} n INNER JOIN {event} e ON n.nid = e.nid INNER JOIN {event_timezones} tz ON tz.timezone = e.timezone WHERE (event_start >= '%s' AND event_start <= '%s') OR (event_end >= '%s' AND event_end <= '%s') OR (event_start <= '%s' AND event_end >= '%s') && n.status = 1 AND ((e.event_start >= '%s' AND e.event_start <= '%s') OR (e.event_end >= '%s' AND e.event_end <= '%s') OR (e.event_start <= '%s' AND e.event_end >= '%s'))";

Line 91: $query = "SELECT n.nid, n.uid, n.title, n.type, e.event_start, e.event_start AS event_start_orig, e.event_end, e.event_end AS event_end_orig, e.timezone, e.has_time, e.has_end_date, tz.offset AS offset, tz.offset_dst AS offset_dst, tz.dst_region, tz.is_dst, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_start_utc, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_end_utc, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_user, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_user, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_site, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_site, tz.name as timezone_name FROM {node} n INNER JOIN {event} e ON n.nid = e.nid INNER JOIN {event_timezones} tz ON tz.timezone = e.timezone WHERE (event_start >= '%s' AND event_start <= '%s') OR (event_end >= '%s' AND event_end <= '%s') OR (event_start <= '%s' AND event_end >= '%s') && n.status = 1 AND ((e.event_start >= '%s' AND e.event_start <= '%s') OR (e.event_end >= '%s' AND e.event_end <= '%s') OR (e.event_start <= '%s' AND e.event_end >= '%s'))";

All you're essentially doing is moving the HAVING clause to the start of the WHERE clause;

HAVING (event_start >= '%s' AND event_start <= '%s') OR (event_end >= '%s' AND event_end <= '%s') OR (event_start <= '%s' AND event_end >= '%s')

So it becomes;

WHERE (event_start >= '%s' AND event_start <= '%s') OR (event_end >= '%s' AND event_end <= '%s') OR (event_start <= '%s' AND event_end >= '%s') && ...rest of query

I've tested it a little and it *seems* fine, obviously that's on a test site though not a production one. I'd appreciate other people testing it too.

Thanks,

Pobster

The issue only occurs if the HAVING does not have a preceding GROUP BY. I just keep wondering what the intention was behind using HAVING in addition to WHERE...

Ohhhhhhhhhh well it doesn't matter even more then - I just assumed the GROUP BY clause had been taken out from the last release? Anyways, I've got it live on a production site right now and acl, forum_access and the event module are all playing nicely together. Well... Nicely together after I've fixed the other issue with the template_preprocess as well...

NO errors in my dblog at all, calendar works just fine in all views. Forums private exactly how they should be (one private, one not).

Pobster

Nice to know — thanks for taking the lead! I hope killes will fix this...

This one; http://drupal.org/node/223685

It fixes the display for all calendar views, but well... They're flawed any way as they don't work as they should do. The stripe is borked for one, there's other stuff too... I might fix the other problems as well, dunno really - see how it goes...

edit: did I imagine you asked 'what other issue?'??!!! Or have you edited your post above?!?! Think I'm going crazy!!!

Anyways, as I'm editing this, thought I'd just ask - please if a couple of people could post that this code change works fine then change the status to 'reviewed by the community' or whatever it is, that'd be great thanks.

Pobster

I've also been trying to figure this all out, and I really appreciate you working on this pobster and others. I tried changing the HAVING to WHERE. Here's the code. I'm not anywhere near a great coder. Hell I don't even consider myself a coder. But I attempt it when somethings messed up. :) But changing the code from HAVING to WHERE has just caused pretty much the same error (Line 102) instead of the HAVING in the code it's the WHERE. :(

Line: 32 $query = "SELECT n.nid, n.uid, n.title, n.type, e.event_start AS event_start_orig, e.event_end AS event_end_orig, e.timezone, e.has_time, e.has_end_date, tz.offset AS offset, tz.offset_dst AS offset_dst, tz.dst_region, tz.is_dst, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_start_utc, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_end_utc, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_user, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_user, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_site, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_site, tz.name as timezone_name FROM {node} n INNER JOIN {event} e ON n.nid = e.nid INNER JOIN {event_timezones} tz ON tz.timezone = e.timezone WHERE n.status = 1 AND ((e.event_start >= '%s' AND e.event_start <= '%s') OR (e.event_end >= '%s' AND e.event_end <= '%s') OR (e.event_start <= '%s' AND e.event_end >= '%s')) WHERE (event_start >= '%s' AND event_start <= '%s') OR (event_end >= '%s' AND event_end <= '%s') OR (event_start <= '%s' AND event_end >= '%s')";

Line 61: $query = "SELECT n.nid, n.uid, n.title, n.type, e.event_start AS event_start_orig, e.event_end AS event_end_orig, e.timezone, e.has_time, e.has_end_date, tz.offset AS offset, tz.offset_dst AS offset_dst, tz.dst_region, tz.is_dst, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_start_utc, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_end_utc, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_user, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_user, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_site, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_site, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end, tz.name as timezone_name FROM {node} n INNER JOIN {event} e ON n.nid = e.nid INNER JOIN {event_timezones} tz ON tz.timezone = e.timezone WHERE n.status = 1 AND ((e.event_start >= '%s' AND e.event_start <= '%s') OR (e.event_end >= '%s' AND e.event_end <= '%s') OR (e.event_start <= '%s' AND e.event_end >= '%s')) WHERE (event_start >= '%s' AND event_start <= '%s') OR (event_end >= '%s' AND event_end <= '%s') OR (event_start <= '%s' AND event_end >= '%s')";

Line 91: $query = "SELECT n.nid, n.uid, n.title, n.type, e.event_start, e.event_start AS event_start_orig, e.event_end, e.event_end AS event_end_orig, e.timezone, e.has_time, e.has_end_date, tz.offset AS offset, tz.offset_dst AS offset_dst, tz.dst_region, tz.is_dst, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_start_utc, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND AS event_end_utc, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_user, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_user, e.event_start - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_start_site, e.event_end - INTERVAL IF(tz.is_dst, tz.offset_dst, tz.offset) HOUR_SECOND + INTERVAL %d SECOND AS event_end_site, tz.name as timezone_name FROM {node} n INNER JOIN {event} e ON n.nid = e.nid INNER JOIN {event_timezones} tz ON tz.timezone = e.timezone WHERE n.status = 1 AND ((e.event_start >= '%s' AND e.event_start <= '%s') OR (e.event_end >= '%s' AND e.event_end <= '%s') OR (e.event_start <= '%s' AND e.event_end >= '%s')) WHERE (event_start >= '%s' AND event_start <= '%s') OR (event_end >= '%s' AND event_end <= '%s') OR (event_start <= '%s' AND event_end >= '%s')";

I'm having the same issues as you guys, but instead of ACL it's node_by_role_permissions. This is also for the event_database.mysql.inc file. not the sqli file.

No read what I wrote again, you need to MOVE the HAVING clause to the start of the WHERE clause, you don't make it into a new WHERE clause - that doesn't make any sense? Just cut and paste my lines above if you're not sure, else read the instructions closely in #4.

The reasoning behind this is as easy as speaking English;

Only do THIS WHERE something is TRUE whilst also HAVING cake to eat.

So it can also be written as;

Only do THIS WHERE I'm eating cake AND something is TRUE

Pobster

Awesome. It works. You're the best!

Now I'm just having issues w/ the calendar block displaying on the bottom left of the page, instead of it sitting on the right side like it's supposed to. IE6 Issue.

Again, thank you very much. :)

there's no patch here.

removing the having clause (and moving it's arguments into WHERE) is not an option as the SQL is different.

IMO this is a core bug.

Ok, can anybody who has this problem add "GROUP BY n.nid " just before the HAVING part?

I need more info on how to replicate this bug, acl.module alone doesn't seem to suffice as it doesn't implement the rewrite hook.

Activate acl and forum access, you obviously don't get the error when you're logged in as superuser as you override any permissions you set. It's very easy to replicate... Acl is a framework and doesn't actually do anything on its own, hence you need to activate something that actually uses it.

BTW, I marked this as having a patch because it's better for people to see that at least there's a solution for this issue rather than leaving it active (and so it looks like no-ones bothered), there isn't a status for "change these few lines and it'll work"... I didn't make a patch because I've changed so much of the module that the patch would address all of the problems I've been having and wouldn't be specific to this issue (hardly surprising as the module is in development) but it'd also add functionality for custom things I require as well (node_color support) and I'm not bothered about faffing around cleaning up patch files when you'll probably fix this issue next release anyway...

Pobster

I've enabled forum_access and still don't see the error (yes, not user 1)...

Well you know as well as me that's impossible, you know they removed support for 'HAVING' without a preceding 'GROUP BY' see comment #2... You are using the current releases of Drupal/ Event/ ACL/ Forum Access right...? Has this issue already been address in core? Are you using Drupal 6.1?

Pobster

I've no idea why I don't get the bug, I guess I should be seeing it. However, I am very limited in time atm and if you want me to fix it I suggest you try my suggestion in #13 and let me know if we can use this workaround until a proper fix for core has been made available.

I had the bug. Adding "GROUP BY n.nid " just before HAVING removed the bug. Had the same result (in that sense) as the code in #4.

Aye yep, behaves fine with that change - stick it in!

Pobster

Great, I have committed this workaround. I leave this open so I can remove it again once core gets fixed.

There's now a patch available on the core issue:

http://drupal.org/node/151910#comment-800780

Please test it and report success (or failure) there after removing the workaround I committed.

Nope unfortunately it results in a blank calendar as before. Strange too, I highly expected that would work?

Pobster

I see this error with the 5.2 dev version of Event also in drupal 5.7. But just on the event/select view.

    * user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '00:38)) AND (MONTH(event.event_start) >= MONTH(2008-07-09 00:38)) AND (DAYOFMONT' at line 1 query: SELECT count( DISTINCT(node.nid)) FROM node node LEFT JOIN event event ON node.nid = event.nid WHERE (node.status = '1') AND (node.type IN ('event','minutes')) AND (YEAR(event.event_start) = YEAR(2008-07-09 00:38)) AND (MONTH(event.event_start) >= MONTH(2008-07-09 00:38)) AND (DAYOFMONTH(event.event_start) >= 1) in .../includes/database.mysql.inc on line 172.
    * user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '00:38)) AND (MONTH(event.event_start) >= MONTH(2008-07-09 00:38)) AND (DAYOFMONT' at line 1 query: SELECT DISTINCT(node.nid), node.title AS node_title, node.changed AS node_changed, event.event_start AS event_event_start, event.event_end AS event_event_end, event.timezone AS event_timezone FROM node node LEFT JOIN event event ON node.nid = event.nid WHERE (node.status = '1') AND (node.type IN ('event','minutes')) AND (YEAR(event.event_start) = YEAR(2008-07-09 00:38)) AND (MONTH(event.event_start) >= MONTH(2008-07-09 00:38)) AND (DAYOFMONTH(event.event_start) >= 1) ORDER BY node_title ASC LIMIT 0, 10 in .../includes/database.mysql.inc on line 172.

I've added the workaround to the D5.2 branch. Again leaving active.

subscribing

Event for Drupal 8 is unrelated to older versions. If an issue similar to this one exists, please open a new issue with the 8.x branch.