Moodle
  1. Moodle
  2. MDL-30515

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical 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:
    • Rank:
      33210

      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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          Justin Filip added a comment -

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

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

          Thanks for following through on this.

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

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

          Show
          Martin Dougiamas added a comment - This can break upgrades in some cases, so hope it can be put in 2.2.1
          Hide
          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
          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
          David Monllaó added a comment -

          Patch tested with mysql, postgres, oracle and mssql

          Show
          David Monllaó added a comment - Patch tested with mysql, postgres, oracle and mssql
          Hide
          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 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
          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
          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
          David Monllaó added a comment -

          Thanks Aparup, pull branches rebased

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

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

          Show
          Sam Hemelryk added a comment - Thanks, this has been integrated now. Moodle 2.2 only.
          Hide
          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
          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
          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
          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
          Petr Škoda added a comment -

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

          Show
          Petr Škoda added a comment - Hi, this crated a regression for PostgreSQL - see MDL-34840 .
          Hide
          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
          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: