Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.9, 2.0
    • Component/s: Database SQL/XMLDB
    • Labels:
      None
    • Environment:
      MySQL 5, PHP 5, Apache 2
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE

      Description

      While I was browsing through install.php, I've noted a comment saying that Moodle should create its database, but it doesn't. I've looked into adodb an noticed that it caches previous connection parameters (including database name). That is why the second connect fails (it has the same semantics as the first one). I've fixed this problem and
      attached install.php.patch.

        Gliffy Diagrams

        1. install_mixed.patch
          2 kB
          Eloy Lafuente (stronk7)
        2. install.php.patch
          2 kB
          Andrei Bautu
        3. mdl_9609_install.php.diff
          2 kB
          Anthony Borrow

          Activity

          Hide
          aborrow Anthony Borrow added a comment -

          The patch assumes that the default mysql database exists. I chose it so that ADODB would have a default database to connect to. I was then able to use the case scenario to create the new database and connect to it. Note that I also specified the utf8 character set and the utf8_general_ci collation in order to ensure that the utf8 compatibility check will pass. Hopefully this will help resolve this issue. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - The patch assumes that the default mysql database exists. I chose it so that ADODB would have a default database to connect to. I was then able to use the case scenario to create the new database and connect to it. Note that I also specified the utf8 character set and the utf8_general_ci collation in order to ensure that the utf8 compatibility check will pass. Hopefully this will help resolve this issue. Peace - Anthony
          Hide
          aborrow Anthony Borrow added a comment -

          Eloy - Because of the impact that this could have in simplifying installs and because I think that it is easily resolvable (especially for 1.9) I am changing the priority of this to critical. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Eloy - Because of the impact that this could have in simplifying installs and because I think that it is easily resolvable (especially for 1.9) I am changing the priority of this to critical. Peace - Anthony
          Hide
          aborrow Anthony Borrow added a comment -

          Eloy - Just to clarify, by default mysql database I literally mean the mysql database named mysql. I would be interested on your thoughts for this patch. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Eloy - Just to clarify, by default mysql database I literally mean the mysql database named mysql. I would be interested on your thoughts for this patch. Peace - Anthony
          Hide
          aborrow Anthony Borrow added a comment -

          Actually I should have said to look at the diff file (mdl_9609_install.php.diff ) that I uploaded. The file labeled patch is not the fix that I am proposing and was created by Andrei.

          Show
          aborrow Anthony Borrow added a comment - Actually I should have said to look at the diff file (mdl_9609_install.php.diff ) that I uploaded. The file labeled patch is not the fix that I am proposing and was created by Andrei.
          Hide
          abautu Andrei Bautu added a comment -

          Hi Anthony,

          The problem with your solution is that you assume that the user has access to the "mysql" database. You might have this privilege if you're the server administrator or in a similar context, but most providers do not allow you to connect to this database.

          Cheers,
          Andrei

          Show
          abautu Andrei Bautu added a comment - Hi Anthony, The problem with your solution is that you assume that the user has access to the "mysql" database. You might have this privilege if you're the server administrator or in a similar context, but most providers do not allow you to connect to this database. Cheers, Andrei
          Hide
          aborrow Anthony Borrow added a comment -

          Andrei - I could see where that might be the case. I thought I had tested this with hostmonster.com and it worked (as I recall). What about using the information_schema? Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Andrei - I could see where that might be the case. I thought I had tested this with hostmonster.com and it worked (as I recall). What about using the information_schema? Peace - Anthony
          Hide
          aborrow Anthony Borrow added a comment -

          What would be a good way to create a database user account with minimal privileges that you might expect to find on a host? I'll try working with that. My other thought is that this is at least a step in the right direction and if it fails for some then they will have to create the database manually; however, if we can get it working for most then all the better. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - What would be a good way to create a database user account with minimal privileges that you might expect to find on a host? I'll try working with that. My other thought is that this is at least a step in the right direction and if it fails for some then they will have to create the database manually; however, if we can get it working for most then all the better. Peace - Anthony
          Hide
          abautu Andrei Bautu added a comment - - edited

          Anthony, I'm affraid information_schema won't work (i believe this database it's even more "private" to mysql than the "mysql" database itself). I think the best way to do it is to connect to the mysql server without selecting any database (because it might be none). That's what my patch does. Did you see any problem with it?

          Show
          abautu Andrei Bautu added a comment - - edited Anthony, I'm affraid information_schema won't work (i believe this database it's even more "private" to mysql than the "mysql" database itself). I think the best way to do it is to connect to the mysql server without selecting any database (because it might be none). That's what my patch does. Did you see any problem with it?
          Hide
          aborrow Anthony Borrow added a comment -

          Andrei - From what I could tell you have to be connected to a database in order for the create database command to work. I'm surprised that connecting to the mysql or information_schema would be a problem. What rights are minimally required to create a database? I would expect the user to have those rights. If they do not have those rights then they would not be able to create it manually. I do not recall what difficulty I ran into with your patch. I do recall trying it but I'll have to go back and try it again to see what happened. Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - Andrei - From what I could tell you have to be connected to a database in order for the create database command to work. I'm surprised that connecting to the mysql or information_schema would be a problem. What rights are minimally required to create a database? I would expect the user to have those rights. If they do not have those rights then they would not be able to create it manually. I do not recall what difficulty I ran into with your patch. I do recall trying it but I'll have to go back and try it again to see what happened. Peace - Anthony
          Hide
          abautu Andrei Bautu added a comment -

          It's enough to be connected to the server (without selecting any database) in order to create a database. So it's possible to connect to the server only, then create the database and then use that database.

          Regarding the create rights, a user may create a database if it has enough right to perform a 'create database' command on that database name (ie. if (s)he has CREATE privilege for the database).

          For example, if I get an account (named abautu) at a hosting provider that also gives me one database (named abautu_db), it is likely that the administrator issues a command like:
          GRANT ALL ON abautu_db.* TO 'abautu'@'localhost' IDENTIFIED BY PASSWORD '1234';

          Now, I can connect to the server (not to the database, because it does not exist yet) and create the database with:
          CREATE DATABASE abautu_db;

          Cheers,
          Andrei

          Show
          abautu Andrei Bautu added a comment - It's enough to be connected to the server (without selecting any database) in order to create a database. So it's possible to connect to the server only, then create the database and then use that database. Regarding the create rights, a user may create a database if it has enough right to perform a 'create database' command on that database name (ie. if (s)he has CREATE privilege for the database). For example, if I get an account (named abautu) at a hosting provider that also gives me one database (named abautu_db), it is likely that the administrator issues a command like: GRANT ALL ON abautu_db.* TO 'abautu'@'localhost' IDENTIFIED BY PASSWORD '1234'; Now, I can connect to the server (not to the database, because it does not exist yet) and create the database with: CREATE DATABASE abautu_db; Cheers, Andrei
          Hide
          dougiamas Martin Dougiamas added a comment -

          I think this looks OK, as long as it:

          • only tries to create a database when one can connect to the server but no database exists yet
          • gives a useful warning when the database creation fails

          Eloy, can you give a final +10?

          Show
          dougiamas Martin Dougiamas added a comment - I think this looks OK, as long as it: only tries to create a database when one can connect to the server but no database exists yet gives a useful warning when the database creation fails Eloy, can you give a final +10?
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Well,

          really both patches (Andrei and Anthony) have cool ideas to be implemented:

          1) Andrei's tip to reset database name and be able to reconnect without DB.
          2) Anthony character set specification.

          So, I've reviewed both patches and they are really similar in concept and both are pretty EQUIVALENT to code currently in install.php, i.e. right now, the database is created if that's possible (mysql and mysqli drivers).

          So, I've added the cool 1) and 2) ideas above and attached it as a really simple "install_mixed.patch" change. I think it's the definitive patch that we should apply. Simpler and with the best of both worlds. Tested here under "mysqli" (should work exactly the same under old "mysql" driver). It informs about error creating DB and so on, as requested.

          Please take a look. We could extend it in the future to support other DBs but for now I'd leave if only for the 2 MySQL flavours.

          Ciao

          PS: If nothing arrives against it in the next, say 12 hours, I'll send this to 19_STABLE and HEAD.

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Well, really both patches (Andrei and Anthony) have cool ideas to be implemented: 1) Andrei's tip to reset database name and be able to reconnect without DB. 2) Anthony character set specification. So, I've reviewed both patches and they are really similar in concept and both are pretty EQUIVALENT to code currently in install.php, i.e. right now, the database is created if that's possible (mysql and mysqli drivers). So, I've added the cool 1) and 2) ideas above and attached it as a really simple "install_mixed.patch" change. I think it's the definitive patch that we should apply. Simpler and with the best of both worlds. Tested here under "mysqli" (should work exactly the same under old "mysql" driver). It informs about error creating DB and so on, as requested. Please take a look. We could extend it in the future to support other DBs but for now I'd leave if only for the 2 MySQL flavours. Ciao PS: If nothing arrives against it in the next, say 12 hours, I'll send this to 19_STABLE and HEAD.
          Hide
          aborrow Anthony Borrow added a comment -

          +1 - Sounds like the best of both worlds! Peace - Anthony

          Show
          aborrow Anthony Borrow added a comment - +1 - Sounds like the best of both worlds! Peace - Anthony
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Done. Changes are in CVS (both 19_STABLE and HEAD). Also, I've added one new string "databasesettingswillbecreated" to be noted about the automatic DB creation that will happen with MySQL.

          Closing, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Done. Changes are in CVS (both 19_STABLE and HEAD). Also, I've added one new string "databasesettingswillbecreated" to be noted about the automatic DB creation that will happen with MySQL. Closing, ciao
          Hide
          tsala Helen Foster added a comment -

          Hi Eloy,

          Thanks for fixing this bug

          Anthony suggested to me that http://docs.moodle.org/en/Installing_Moodle may need to be updated to indicate that the step of creating the database is optional.

          Please could you clarify which databases are created automatically - is it only MySQL?

          Show
          tsala Helen Foster added a comment - Hi Eloy, Thanks for fixing this bug Anthony suggested to me that http://docs.moodle.org/en/Installing_Moodle may need to be updated to indicate that the step of creating the database is optional. Please could you clarify which databases are created automatically - is it only MySQL?

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                3/Mar/08