As per title.

CommentFileSizeAuthor
#7 signup_mysql5.0_fix.patch1017 bytesdww

Comments

hunmonk’s picture

Status: Active » Closed (fixed)

i checked into this, and since the table isn't generated from a single query, you can't use drupal's tablesorting functionality to sort by anything not in the main query--which means we can't sort by number of signups. i don't like the idea of making it sortable by only two of the three parameters--i'm open to suggestions about how to make it work w/ the tablesort functionality, but i'm not seeing it right now.

boris mann’s picture

Status: Closed (fixed) » Active

You could just use links instead, that alter the actual query -- i.e. underneath Signups, have local tasks labelled "by date" (which is the default"), "by no. of signups", and "by title".

Sorting by number of signups is quite useful, especially when you could have a site with 100s of signups. I could probably come up with other features, like exporting to CSV, which is really necessary for things like arbitrary data fields and having print outs / desktop sorting etc.

hunmonk’s picture

Version: 4.6.x-1.x-dev » 5.x-2.x-dev
Assigned: Unassigned » hunmonk
Status: Active » Fixed

heh. or maybe i could figure out another way to write the query, which i just did... :)

feature has been applied to 4.6 and HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)
dww’s picture

Version: 5.x-2.x-dev » 4.6.x-1.x-dev
Status: Closed (fixed) » Active

sorry to be the bearer of bad news, but this change (revision 1.1.2.19) breaks the admin/signups page for sites where the MySQL server is 5.0.12 or higher. :( when i use revision 1.1.2.19 or later from the 4.6 branch, i get this at the top of the page, and no entries in the table:

user error: Unknown column 's.nid' in 'on clause'
query: SELECT COUNT(*) FROM signup_log s_l INNER JOIN node n ON
    n.nid = s.nid LEFT JOIN event e ON e.nid = n.nid INNER JOIN signup s ON s.nid = s_l.nid WHERE s.completed = 0 GROUP BY n.nid  in /home/.lawton/dww/drupal/drupal-4.6.5/includes/database.mysql.inc on line 66.

user error: Unknown column 's.nid' in 'on clause'
query: SELECT n.title, n.nid, e.event_start, e.timezone, COUNT(*) AS count FROM signup_log s_l INNER JOIN node n ON
    n.nid = s.nid LEFT JOIN event e ON e.nid = n.nid INNER JOIN signup s ON s.nid = s_l.nid WHERE s.completed = 0 GROUP BY n.nid ORDER BY  n.title ASC LIMIT 0, 25 in /home/.lawton/dww/drupal/drupal-4.6.5/includes/database.mysql.inc on line 66.

if i revert to 1.1.2.18, everything's fine. http://drupal.org/node/40623 talks about the same problem. i honestly don't know enough about writing SQL queries to understand the root of the problem, but whatever you changed with revision 1.1.2.19 doesn't work with MySQL 5.0.12 or later...

let me know if i can provide any additional info to help resolve this.

hunmonk’s picture

that error doesn't make sense to me. can you confirm this problem on more than just your mysql installation??

dww’s picture

Status: Active » Needs review
StatusFileSize
new1017 bytes

i just did my homework: it's definitely not just my MySQL install. there are a number of issues lurking around in the drupal forums for other modules with the same problem. it all boils down to the this change from MySQL 5.0.12:

http://dev.mysql.com/doc/refman/5.0/en/join.html

Note: Beginning with MySQL 5.0.12, natural joins and joins with USING, including outer join variants, are processed according to the SQL:2003 standard. These changes make MySQL more compliant with standard SQL. However, they can result in different output columns for some joins. Also, some queries that appeared to work correctly in older versions must be rewritten to comply with the standard. The following list provides more detail about several effects of the 5.0.12 change in join processing. The term “previously” means “prior to MySQL 5.0.12.”

if you search further down on that page, you get to this gem:

Previously, the ON clause could refer to columns in tables named to its right. Now an ON clause can refer only to its operands.

Example:

CREATE TABLE t1 (i1 INT);
CREATE TABLE t2 (i2 INT);
CREATE TABLE t3 (i3 INT);
SELECT * FROM t1 JOIN t2 ON (i1 = i3) JOIN t3;

Previously, the SELECT statement was legal. Now the statement fails with an Unknown column 'i3' in 'on clause' error
because i3 is a column in t3, which is not an operand of the ON clause. The statement should be rewritten as follows:

SELECT * FROM t1 JOIN t2 JOIN t3 ON (i1 = i3);

the problem with signup.module revision 1.1.2.19 is this:

$sql = "SELECT n.title, n.nid$event_select, COUNT(*) AS count FROM {signup_log} s_l INNER JOIN {node} n ON
    n.nid = s.nid$event_join INNER JOIN {signup} s ON s.nid = s_l.nid WHERE s.completed = 0 GROUP BY n.nid".

here, you've violated the SQL-standard behavior of the ON clause quoted above, since you refer to "s.nid" before you "JOIN {signup} s"...

here's how i've re-written the query, and it works fine on my test site. i don't have a way to test on an older MySQL installation (i don't have as much control over my hosting site as i'd like, and i haven't gone through the trouble of setting up my own laptop as a test web server), but i think it should work:

  $sql = "SELECT n.title, n.nid$event_select, COUNT(*) AS count FROM
          {signup_log} s_l INNER JOIN {node} n INNER JOIN {signup} s
          ON n.nid = s.nid AND s.nid = s_l.nid$event_join
          WHERE s.completed = 0 GROUP BY n.nid";
  $sql .= tablesort_sql($header);

i'm not an SQL expert at all, so i'm not 100% sure this is semantically equivalent to what you had, but it appears to work with limited testing.

once i apply this patch, my 1.1.2.22 version of signup.module works fine, including the admin/signup page, i can see all the events with open signups, re-sort the table, etc.

let me know if you need any other info.
-derek

hunmonk’s picture

Version: 4.6.x-1.x-dev » 5.x-2.x-dev
Category: feature » bug
Status: Needs review » Fixed

applied to 4.6 and HEAD. nice work!

__Tango’s picture

Status: Fixed » Active

Unfortunately, i think this new sql query breaks on mysql 3. I'm running 3.23.58 and i using the new query:

SELECT n.title, n.nid, COUNT(*) AS count FROM
          signup_log s_l INNER JOIN node n INNER JOIN signup s
          ON n.nid = s.nid AND s.nid = s_l.nid
          WHERE s.completed = 0 GROUP BY n.nid

I get the error:

[Error Code: 1064, SQL State: 42000]  Syntax error or access violation message from server: 
"You have an error in your SQL syntax near 'INNER JOIN signup s ON n.nid = s.nid AND s.nid = s_l.nid W' at line 1

As expected, the old query works fine.

hunmonk’s picture

unless we can find a query that will work on mysql 3.x 4.x and 5.x, i'm not going to change the query, as we will need 5.x compatibility going forward, and 3.x compatibility will go away at some point--iirc there's already been talk of us dropping support for 3.x

if you find a query that works on all three, please submit a patch. otherwise, i think i'm going to leave it as is.

__Tango’s picture

hunmonk, killes, and I worked a bit on this this morning. hunmonk came up with a new query that should be ansi sql compliant and thus should work in all versions of mysql.

The query is:

$sql = "SELECT n.title, n.nid$event_select, COUNT(*) AS count FROM {signup} s 
  INNER JOIN {node} n ON n.nid = s.nid INNER JOIN {signup_log} s_l
  ON s.nid = s_l.nid$event_join WHERE s.completed = 0 GROUP BY n.nid";

This query has been tested against mysql-3.23.58 and mysql4. We need someone with mysql5 (something later than 5.0.12) to test this. if you can please give it a shot and report back here if it works.

Thanks.

...alex...

dww’s picture

tested on MySQL 5.0.18. works fine. thanks for fixing my fix!

hunmonk’s picture

Status: Active » Fixed

fixed in 4.6 and lastest CVS. nice work, guys!

Anonymous’s picture

Status: Fixed » Closed (fixed)