Moodle
  1. Moodle
  2. MDL-28537

Wrong MySQL syntax causes error while synchronizing LDAP users

    Details

    • Database:
      MySQL
    • Testing Instructions:
      Hide

      Run Moodle 1.9.12 with MySQL version where 'TYPE' not supported as table_option
      Configure LDAP Authentication
      Run the synchronization script "moodle\auth\ldap\auth_ldap_sync_users.php"

      Show
      Run Moodle 1.9.12 with MySQL version where 'TYPE' not supported as table_option Configure LDAP Authentication Run the synchronization script "moodle\auth\ldap\auth_ldap_sync_users.php"
    • Workaround:
      Hide

      Modify line #566 of script "moodle\auth\ldap\auth.php", because in MySQL 5.5 doesn't support "type" table option, instead it uses ENGINE option.

      From:
      $createtemptablesql = 'CREATE TEMPORARY TABLE ' . $temptable . ' (username VARCHAR(64), PRIMARY KEY (username)) TYPE=MyISAM';

      To:
      $createtemptablesql = 'CREATE TEMPORARY TABLE ' . $temptable . ' (username VARCHAR(64), PRIMARY KEY (username)) ENGINE=MyISAM';

      Show
      Modify line #566 of script "moodle\auth\ldap\auth.php", because in MySQL 5.5 doesn't support "type" table option, instead it uses ENGINE option. From: $createtemptablesql = 'CREATE TEMPORARY TABLE ' . $temptable . ' (username VARCHAR(64), PRIMARY KEY (username)) TYPE=MyISAM'; To: $createtemptablesql = 'CREATE TEMPORARY TABLE ' . $temptable . ' (username VARCHAR(64), PRIMARY KEY (username)) ENGINE=MyISAM';
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      18194

      Description

      I'm running Moodle 1.9.12+ (Build: 20110701) with MySQL 5.5 and after configuring LDAP Authentication i try to execute the "moodle\auth\ldap\auth_ldap_sync_users.php" script to synchronize my users. But the following error is shown:

      Configuring temp table
      Creating temp table mdl_extuser
      Failed to create temporary users table - aborting

      The problem is in line #566 of script "moodle\auth\ldap\auth.php", because in MySQL 5.5 doesn't support "type" table option, instead it uses ENGINE option. So i changed the code from:
      $createtemptablesql = 'CREATE TEMPORARY TABLE ' . $temptable . ' (username VARCHAR(64), PRIMARY KEY (username)) TYPE=MyISAM';

      To:
      $createtemptablesql = 'CREATE TEMPORARY TABLE ' . $temptable . ' (username VARCHAR(64), PRIMARY KEY (username)) ENGINE=MyISAM';

      But it will probably fail on MySQL older versions.

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this and providing a solution.

        Show
        Michael de Raadt added a comment - Thanks for reporting this and providing a solution.
        Hide
        Michael de Raadt added a comment -

        This affects 1.9 only.

        Show
        Michael de Raadt added a comment - This affects 1.9 only.
        Hide
        Sam Hemelryk added a comment -

        Hi guys,

        A bit of research shows that support for the TYPE table option to specify the storage engine was dropped in 5.1, from that version onwards you should use ENGINE instead.
        The ENGINE option was added in 4.1.2 and given that Moodle 1.9.13+ requires MySQL >= 4.1.16.

        Because of this I see no problem in changing over the syntax here, and of course it only applies to the 1.9 branch.

        I'll create a patch for this shortly.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi guys, A bit of research shows that support for the TYPE table option to specify the storage engine was dropped in 5.1, from that version onwards you should use ENGINE instead. The ENGINE option was added in 4.1.2 and given that Moodle 1.9.13+ requires MySQL >= 4.1.16. Because of this I see no problem in changing over the syntax here, and of course it only applies to the 1.9 branch. I'll create a patch for this shortly. Cheers Sam
        Hide
        Aparup Banerjee added a comment -

        This change looks good to me.
        +1 to make this consistent elsewhere in 1.9 where create table still uses 'TYPE' as table_option. (grep shows quiet a few within */db/mysql.php)

        Show
        Aparup Banerjee added a comment - This change looks good to me. +1 to make this consistent elsewhere in 1.9 where create table still uses 'TYPE' as table_option. (grep shows quiet a few within */db/mysql.php)
        Hide
        Sam Hemelryk added a comment -

        Thanks Apu, I've made the change consistent and put up for integration.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Thanks Apu, I've made the change consistent and put up for integration. Cheers Sam
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Correct, TYPE is deprecated since 4.0, throws warnings since 5.1.8 and was removed completely in 5.5 (http://dev.mysql.com/doc/refman/5.1/en/news-5-1-8.html) so the change is ok. Integrating.. thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Correct, TYPE is deprecated since 4.0, throws warnings since 5.1.8 and was removed completely in 5.5 ( http://dev.mysql.com/doc/refman/5.1/en/news-5-1-8.html ) so the change is ok. Integrating.. thanks!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I've added one extra commit cleaning 1-2 wrong whitespace lines, just for being able to check some CI tests are working ok here for 19_STABLE.

        Perhaps this would be worth being re-titled and pushed to 1.9.14 release notes as "Support for MySQL 5.5 improved" or so? Just an idea.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - I've added one extra commit cleaning 1-2 wrong whitespace lines, just for being able to check some CI tests are working ok here for 19_STABLE. Perhaps this would be worth being re-titled and pushed to 1.9.14 release notes as "Support for MySQL 5.5 improved" or so? Just an idea. Ciao
        Hide
        Aparup Banerjee added a comment -

        works for me. fresh installation has not problems as well.

        Show
        Aparup Banerjee added a comment - works for me. fresh installation has not problems as well.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay!

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - git & cvs repositories have been populated with this solution. Many thanks for your collaboration, yay! Closing, ciao

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: