Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-35080

Detect foreign tables in 2.3.x upgrade, print error and skip

          Details

          • Type: Improvement
          • Status: Closed
          • Priority: Minor
          • Resolution: Fixed
          • Affects Version/s: 2.3.1, 2.4
          • Fix Version/s: 2.3.2
          • Component/s: Database SQL/XMLDB
          • Labels:
          • Testing Instructions:
            Hide

            1/ install Moodle 2.2 with 'mdl_' prefix
            2/ add bogus tables with names: 'mdl_111user', 'mdl_user-bak', 'mdl_userBak'
            3/ create with 'mdl_tableview' from any other table such as "create view mdl_tableview as select * from mdl_config"
            4/ expected warnings during upgrade to 2.3 or master branch:

            • Database table with invalid name 'mdl_111user' detected, skipping.
            • Database table with invalid name 'mdl_user-bak' detected, skipping.
            • Database table with invalid name 'mdl_userBak' detected, skipping.
            • Unsupported database table view 'mdl_tableview' detected, skipping.
            Show
            1/ install Moodle 2.2 with 'mdl_' prefix 2/ add bogus tables with names: 'mdl_111user', 'mdl_user-bak', 'mdl_userBak' 3/ create with 'mdl_tableview' from any other table such as "create view mdl_tableview as select * from mdl_config" 4/ expected warnings during upgrade to 2.3 or master branch: Database table with invalid name 'mdl_111user' detected, skipping. Database table with invalid name 'mdl_user-bak' detected, skipping. Database table with invalid name 'mdl_userBak' detected, skipping. Unsupported database table view 'mdl_tableview' detected, skipping.
          • Affected Branches:
            MOODLE_23_STABLE, MOODLE_24_STABLE
          • Fixed Branches:
            MOODLE_23_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            w36_MDL-35080_m24_weirdtables
          • Pull Master Diff URL:

            Description

            It is not allowed to add custom views or tables that are not defined in install.xml, these foreign tables must not use moodle database table prefix.

            Some admins and developers ignore this, we need to educate them and for now skip these tables during upgrade to 2.3.x

            In the future standalone db diagnostics scripts should detect this and warn admins.

              Gliffy Diagrams

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  skodak Petr Skoda added a comment -

                  see:

                  Show
                  skodak Petr Skoda added a comment - see: http://moodle.org/mod/forum/discuss.php?d=209865 http://moodle.org/mod/forum/discuss.php?d=209418
                  Hide
                  Permalink
                  nebgor Aparup Banerjee added a comment -

                  Thanks Petr, that's been integrated into 23 and master.

                  Show
                  nebgor Aparup Banerjee added a comment - Thanks Petr, that's been integrated into 23 and master.
                  Hide
                  Permalink
                  salvetore Michael de Raadt added a comment -

                  I tested this on upgrades using Oracle, MSSQL, PostgreSQL and MySQL.

                  In Oracle, I didn't use mdl_ as a prefix, but I don't think that matters. I was not able to create a table called c_user-bak; Oracle prevented me from doing this. Also, the c_userBak table was shown by Oracle as C_USERBAK, so it is probably not invalid there. When I ran the upgrade in Oracle it didn't do the field upgrade, so I didn't see any warnings. The upgrade proceeded without any problems.

                  In MySQL, the table mdl_userBak was renamed to mdl_userbak. I saw the following warnings...

                  Database table with invalid name 'mdl_111user' detected, skipping.
                  Unsupported database table view 'mdl_tableview' detected, skipping.
                  Database table with invalid name 'mdl_user-bak' detected, skipping.

                  ...so all good there.

                  In MSSQL, the DB accepted all table names without any trouble. When I ran the upgrade it didn't do the field upgrade, so no warnings there either. The upgrade ran smoothly still.

                  PostgreSQL also accepted the tables without question. It didn't upgrade fields either. The upgrade still ran without issue.

                  Show
                  salvetore Michael de Raadt added a comment - I tested this on upgrades using Oracle, MSSQL, PostgreSQL and MySQL. In Oracle, I didn't use mdl_ as a prefix, but I don't think that matters. I was not able to create a table called c_user-bak; Oracle prevented me from doing this. Also, the c_userBak table was shown by Oracle as C_USERBAK, so it is probably not invalid there. When I ran the upgrade in Oracle it didn't do the field upgrade, so I didn't see any warnings. The upgrade proceeded without any problems. In MySQL, the table mdl_userBak was renamed to mdl_userbak. I saw the following warnings... Database table with invalid name 'mdl_111user' detected, skipping. Unsupported database table view 'mdl_tableview' detected, skipping. Database table with invalid name 'mdl_user-bak' detected, skipping. ...so all good there. In MSSQL, the DB accepted all table names without any trouble. When I ran the upgrade it didn't do the field upgrade, so no warnings there either. The upgrade ran smoothly still. PostgreSQL also accepted the tables without question. It didn't upgrade fields either. The upgrade still ran without issue.
                  Hide
                  Permalink
                  salvetore Michael de Raadt added a comment -

                  Test result: Success!

                  Show
                  salvetore Michael de Raadt added a comment - Test result: Success!
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  Many thanks for the hard work.

                  These changes have been spread upstream and are already available in the git and cvs repositories.

                  Ciao

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao

                    People

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

                      Dates

                      • Created:
                        Updated:
                        Resolved:
                        Fix Release Date:
                        10/Sep/12