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

      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.

        Gliffy Diagrams

          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: