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

Check for non-unique values before adding unique indexes during Moodle 2.x upgrade

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0, 2.1
    • Fix Version/s: 2.2.5
    • Component/s: Database SQL/XMLDB
    • Environment:
      Probably shouldn't matter but I'm testing on Debian GNU/Linux with MySQL 5.1 and PostgreSQL 9.1
    • Database:
      Any
    • Testing Instructions:
      Hide

      Steps

      1. setup a new Moodle 1.9 site or use an existing one
      2. forcibly insert duplicate records in the mdl_grade_letters table (duplicate values being the same value for contextid, lowerboundary, and letter in two or more records)
      3. perform an upgrade to Moodle 2.2 with the patch applied

      Expected outcome

      The upgrade should complete without any problems.

      Problem outcome

      The upgrade fails when trying to add the unique index on the mdl_grade_letters table.

      Show
      Steps setup a new Moodle 1.9 site or use an existing one forcibly insert duplicate records in the mdl_grade_letters table (duplicate values being the same value for contextid , lowerboundary , and letter in two or more records) perform an upgrade to Moodle 2.2 with the patch applied Expected outcome The upgrade should complete without any problems. Problem outcome The upgrade fails when trying to add the unique index on the mdl_grade_letters table.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:

      Description

      During an upgrade from Moodle 1.9 to Moodle 2.x a unique index is added to the mdl_grade_letters table. I have seen more than one case where this table contains duplicate records that prevent this index from being added and thus halt the upgrade, preventing it from continuing until the duplicates are dealt with.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              jfilip Justin Filip added a comment - - edited

              The unique index was added as part of MDL-11888 and duplicates are a problem despite the comments in there.

              Show
              jfilip Justin Filip added a comment - - edited The unique index was added as part of MDL-11888 and duplicates are a problem despite the comments in there.
              Hide
              jfilip Justin Filip added a comment - - edited

              The code that is trying to insert duplicate records in the mdl_grade_letters table still exists in Moodle 2.x as reported by MDL-29598.

              Show
              jfilip Justin Filip added a comment - - edited The code that is trying to insert duplicate records in the mdl_grade_letters table still exists in Moodle 2.x as reported by MDL-29598 .
              Hide
              jfilip Justin Filip added a comment -

              I have code that will check for the existence of duplicates and remove them that can be added to the upgrade code before the unique index is created. Just putting said code into a Github branch and I will update this issue when that is ready.

              Show
              jfilip Justin Filip added a comment - I have code that will check for the existence of duplicates and remove them that can be added to the upgrade code before the unique index is created. Just putting said code into a Github branch and I will update this issue when that is ready.
              Hide
              jfilip Justin Filip added a comment -

              Attaching a CSV file of duplicate mdl_grade_letters values from a real production site.

              These values can be inserted into the mdl_grade_letters table for testing purposes.

              Show
              jfilip Justin Filip added a comment - Attaching a CSV file of duplicate mdl_grade_letters values from a real production site. These values can be inserted into the mdl_grade_letters table for testing purposes.
              Hide
              jfilip Justin Filip added a comment -

              Added Github pull branches and diff URLs for Moodle 2.0, 2.1, and 2.2 (master).

              Show
              jfilip Justin Filip added a comment - Added Github pull branches and diff URLs for Moodle 2.0, 2.1, and 2.2 (master).
              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for following through on this.

              Show
              salvetore Michael de Raadt added a comment - Thanks for following through on this.
              Hide
              dougiamas Martin Dougiamas added a comment -

              This can break upgrades in some cases, so hope it can be put in 2.2.1

              Show
              dougiamas Martin Dougiamas added a comment - This can break upgrades in some cases, so hope it can be put in 2.2.1
              Hide
              dmonllao David Monllaó added a comment - - edited

              Thanks for the patch Justin, I've changed the process to avoid the creation of a tmp table and just remove the duplicate entries leaving only the newest one.

              Submitting for peer review

              Show
              dmonllao David Monllaó added a comment - - edited Thanks for the patch Justin, I've changed the process to avoid the creation of a tmp table and just remove the duplicate entries leaving only the newest one. Submitting for peer review
              Hide
              dmonllao David Monllaó added a comment -

              Patch tested with mysql, postgres, oracle and mssql

              Show
              dmonllao David Monllaó added a comment - Patch tested with mysql, postgres, oracle and mssql
              Hide
              ankit_frenz Ankit Agarwal added a comment - - edited

              Hi guys,
              The patch looks good to me.
              The only thing I was worried about was reference to these entries being used in other tables,
              but after a bit of research it doesnt look like it is referenced from other tables.

              So my +1 for the patch.

              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - - edited Hi guys, The patch looks good to me. The only thing I was worried about was reference to these entries being used in other tables, but after a bit of research it doesnt look like it is referenced from other tables. So my +1 for the patch. Thanks
              Hide
              nebgor Aparup Banerjee added a comment -

              The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

              TIA and ciao

              Show
              nebgor Aparup Banerjee added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
              Hide
              dmonllao David Monllaó added a comment -

              Thanks Aparup, pull branches rebased

              Show
              dmonllao David Monllaó added a comment - Thanks Aparup, pull branches rebased
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks, this has been integrated now. Moodle 2.2 only.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks, this has been integrated now. Moodle 2.2 only.
              Hide
              salvetore Michael de Raadt added a comment -

              Test result: Success

              Without the change, the update failed. With the change, the update succeeded.

              The duplicate grade letter was consolidated into one entry.

              Show
              salvetore Michael de Raadt added a comment - Test result: Success Without the change, the update failed. With the change, the update succeeded. The duplicate grade letter was consolidated into one entry.
              Hide
              poltawski Dan Poltawski added a comment -

              asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ!

              Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!

              Show
              poltawski Dan Poltawski added a comment - asko, Дзякуй, ধন্যবাদ, Благодаря, Gràcies, 感谢, 謝謝, Hvala, Díky, Tak, Bedankt, Tänan, متشکریم, Salamat, Kiitokset, Merci, Grazas, Danke, Ευχαριστώ, આભાર, תודה, धन्यवाद, Köszönjük, Takk fyrir, Terima Kasih, Grazie, ありがとうございます, Рахмет, សូមអរគុណ, 감사합니다, gratiās, Pateicamies, Ačiū, Благодарам, Tēnā koa, Kia Ora Rawa Atu, आभारी आहोत, Талархал, Takk, Dziękuję, Obrigado, Mulţumesc, Engraziel, Спасибо, Fa'afetai, Хвала, Hvala, ස්තූතියි, Vďaka, Hvala, Mahadsanid, Thanks, Gracias, Tack, Salamat, நன்றி, నెనరులు, ขอบคุณค่ะ! Your work has made it into this weeks Moodle release! There are no gold medals available this week - but millions around the world will benefit. Thank you!
              Hide
              skodak Petr Skoda added a comment -

              Hi, this crated a regression for PostgreSQL - see MDL-34840.

              Show
              skodak Petr Skoda added a comment - Hi, this crated a regression for PostgreSQL - see MDL-34840 .
              Hide
              dmonllao David Monllaó added a comment -

              Hi, wondering about how can I skip that I suppose it was copy & pasting the query from the sql consoles... Sorry about that

              Show
              dmonllao David Monllaó added a comment - Hi, wondering about how can I skip that I suppose it was copy & pasting the query from the sql consoles... Sorry about that

                People

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

                  Dates

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