Moodle
  1. Moodle
  2. MDL-30739

Error when upgrading from 1.9.15 to 2.2 with mssql_n or sqlsrv db connection

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.3, 2.2, 2.3
    • Fix Version/s: 2.1.4, 2.2.1
    • Component/s: Questions
    • Labels:
    • Environment:
      IIS 7.0, PHP 5.3.8 using fastcgi on WIndows server 2008 R2, Microsoft SQL server 2008
    • Database:
      Microsoft SQL
    • Testing Instructions:
      Hide

      We need to do this in all supported databases:

      1) In a Moodle 1.9 or 2.0 site, create a course with a quiz and four questions in its question bank.

      • question a) A numerical question with no units.
      • question b) A numerical question with some units.
      • question c) A calculated question with no units.
      • question d) A calculated question with some units.

      2) Upgrade the site to Moodle 2.1 or 2.2. Make sure the upgrade completes with no errors.

      3) Verify that:

      • question a) is set to Unit handling: "Units are not used at all. Only the numerical value is graded."
      • question b) is set to Unit handling: "Units are optional. If a unit is entered, it is used to convert the reponse to Unit 1 before grading."
      • similarly for c) and d)
      Show
      We need to do this in all supported databases: 1) In a Moodle 1.9 or 2.0 site, create a course with a quiz and four questions in its question bank. question a) A numerical question with no units. question b) A numerical question with some units. question c) A calculated question with no units. question d) A calculated question with some units. 2) Upgrade the site to Moodle 2.1 or 2.2. Make sure the upgrade completes with no errors. 3) Verify that: question a) is set to Unit handling: "Units are not used at all. Only the numerical value is graded." question b) is set to Unit handling: "Units are optional. If a unit is entered, it is used to convert the reponse to Unit 1 before grading." similarly for c) and d)
    • Workaround:
      Hide

      This patch fixes it

      --- question/type/numerical/db/upgrade.php      2011-12-14 09:06:41.661913300 -0600
      +++ question/type/numerical/db/upgrade-patched.php      2011-12-14 09:10:27.543499200 -0600
      @@ -70,8 +70,9 @@
       
               // Set a better default for questions without units.
               $DB->execute('
      -                UPDATE {question_numerical_options} qno
      -                   SET showunits = 3
      +                UPDATE qno
      +                   SET qno.showunits = 3
      +                   FROM {question_numerical_options} qno
                        WHERE NOT EXISTS (
                                SELECT 1
                                  FROM {question_numerical_units} qnu
      
      Show
      This patch fixes it --- question/type/numerical/db/upgrade.php 2011-12-14 09:06:41.661913300 -0600 +++ question/type/numerical/db/upgrade-patched.php 2011-12-14 09:10:27.543499200 -0600 @@ -70,8 +70,9 @@ // Set a better default for questions without units. $DB->execute(' - UPDATE {question_numerical_options} qno - SET showunits = 3 + UPDATE qno + SET qno.showunits = 3 + FROM {question_numerical_options} qno WHERE NOT EXISTS ( SELECT 1 FROM {question_numerical_units} qnu
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      33638

      Description

      received the following error
      /admin/index.php?confirmupgrade=1&confirmrelease=1&confirmplugincheck=1

      Default exception handler: Error writing to database Debug: Incorrect syntax near 'qno'.
      
                      UPDATE mdl_question_numerical_options qno
                         SET showunits = 3
                       WHERE NOT EXISTS (
                               SELECT 1
                                 FROM mdl_question_numerical_units qnu
                                WHERE qnu.question = qno.question)
      [array (
      )]
      * line 397 of \lib\dml\moodle_database.php: dml_write_exception thrown
      * line 255 of \lib\dml\mssql_native_moodle_database.php: call to moodle_database->query_end()
      * line 668 of \lib\dml\mssql_native_moodle_database.php: call to mssql_native_moodle_database->query_end()
      * line 78 of \question\type\numerical\db\upgrade.php: call to mssql_native_moodle_database->execute()
      * line 385 of \lib\upgradelib.php: call to xmldb_qtype_numerical_upgrade()
      * line 1466 of \lib\upgradelib.php: call to upgrade_plugins()
      * line 317 of \admin\index.php: call to upgrade_noncore()
      

        Issue Links

          Activity

          aria pn created issue -
          Michael de Raadt made changes -
          Field Original Value New Value
          Description received the following error
          /admin/index.php?confirmupgrade=1&confirmrelease=1&confirmplugincheck=1

          Default exception handler: Error writing to database Debug: Incorrect syntax near 'qno'.

                          UPDATE mdl_question_numerical_options qno
                             SET showunits = 3
                           WHERE NOT EXISTS (
                                   SELECT 1
                                     FROM mdl_question_numerical_units qnu
                                    WHERE qnu.question = qno.question)
          [array (
          )]
          * line 397 of \lib\dml\moodle_database.php: dml_write_exception thrown
          * line 255 of \lib\dml\mssql_native_moodle_database.php: call to moodle_database->query_end()
          * line 668 of \lib\dml\mssql_native_moodle_database.php: call to mssql_native_moodle_database->query_end()
          * line 78 of \question\type\numerical\db\upgrade.php: call to mssql_native_moodle_database->execute()
          * line 385 of \lib\upgradelib.php: call to xmldb_qtype_numerical_upgrade()
          * line 1466 of \lib\upgradelib.php: call to upgrade_plugins()
          * line 317 of \admin\index.php: call to upgrade_noncore()
          received the following error
          /admin/index.php?confirmupgrade=1&confirmrelease=1&confirmplugincheck=1

          {noformat}
          Default exception handler: Error writing to database Debug: Incorrect syntax near 'qno'.

                          UPDATE mdl_question_numerical_options qno
                             SET showunits = 3
                           WHERE NOT EXISTS (
                                   SELECT 1
                                     FROM mdl_question_numerical_units qnu
                                    WHERE qnu.question = qno.question)
          [array (
          )]
          * line 397 of \lib\dml\moodle_database.php: dml_write_exception thrown
          * line 255 of \lib\dml\mssql_native_moodle_database.php: call to moodle_database->query_end()
          * line 668 of \lib\dml\mssql_native_moodle_database.php: call to mssql_native_moodle_database->query_end()
          * line 78 of \question\type\numerical\db\upgrade.php: call to mssql_native_moodle_database->execute()
          * line 385 of \lib\upgradelib.php: call to xmldb_qtype_numerical_upgrade()
          * line 1466 of \lib\upgradelib.php: call to upgrade_plugins()
          * line 317 of \admin\index.php: call to upgrade_noncore()
          {noformat}
          Workaround This patch fixes it

          --- question/type/numerical/db/upgrade.php 2011-12-14 09:06:41.661913300 -0600
          +++ question/type/numerical/db/upgrade-patched.php 2011-12-14 09:10:27.543499200 -0600
          @@ -70,8 +70,9 @@
           
                   // Set a better default for questions without units.
                   $DB->execute('
          - UPDATE {question_numerical_options} qno
          - SET showunits = 3
          + UPDATE qno
          + SET qno.showunits = 3
          + FROM {question_numerical_options} qno
                            WHERE NOT EXISTS (
                                    SELECT 1
                                      FROM {question_numerical_units} qnu
          This patch fixes it

          {code}
          --- question/type/numerical/db/upgrade.php 2011-12-14 09:06:41.661913300 -0600
          +++ question/type/numerical/db/upgrade-patched.php 2011-12-14 09:10:27.543499200 -0600
          @@ -70,8 +70,9 @@
           
                   // Set a better default for questions without units.
                   $DB->execute('
          - UPDATE {question_numerical_options} qno
          - SET showunits = 3
          + UPDATE qno
          + SET qno.showunits = 3
          + FROM {question_numerical_options} qno
                            WHERE NOT EXISTS (
                                    SELECT 1
                                      FROM {question_numerical_units} qnu
          {code}
          Component/s Questions [ 10087 ]
          Component/s Administration [ 10050 ]
          Michael de Raadt made changes -
          Assignee Rajesh Taneja [ rajeshtaneja ] Tim Hunt [ timhunt ]
          Hide
          Tim Hunt added a comment -

          Eloy, help!

          Two things I don't understand:

          1. Surely many other people have upgraded 1.9 -> 2.0 already on MS SQL, so why is this problem only being found now?

          2. How can I write a query like this so that it works on all our supported DBs?

          Show
          Tim Hunt added a comment - Eloy, help! Two things I don't understand: 1. Surely many other people have upgraded 1.9 -> 2.0 already on MS SQL, so why is this problem only being found now? 2. How can I write a query like this so that it works on all our supported DBs?
          Tim Hunt made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Labels triaged
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi,

          if I'm not wrong, the correct way to write one update like that is, simply to follow the rule of avoiding table aliases for the UPDATE statement, so, something like this:

          UPDATE mdl_question_numerical_options
             SET showunits = 3
          WHERE NOT EXISTS (
              SELECT 1
                FROM mdl_question_numerical_units qnu
               WHERE qnu.question = mdl_question_numerical_options.question)
          

          should work in a cross-db way, just launching the query.....

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, if I'm not wrong, the correct way to write one update like that is, simply to follow the rule of avoiding table aliases for the UPDATE statement, so, something like this: UPDATE mdl_question_numerical_options SET showunits = 3 WHERE NOT EXISTS ( SELECT 1 FROM mdl_question_numerical_units qnu WHERE qnu.question = mdl_question_numerical_options.question) should work in a cross-db way, just launching the query.....
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Confirmed, the query above works against the 4 RDBMS.

          Also, this quick grep:

           grep -ri "UPDATE {.*} " * | grep -vi SE
          

          Only reveals 3 uses:

          
          lib/accesslib.php: $updatesql = "UPDATE {context} ct, {context_temp} temp
          lib/accesslib.php: $updatesql = "UPDATE {context} ct
          question/type/numerical/db/upgrade.php: UPDATE {question_numerical_options} qno
          

          with the 2 accesslib ones already being handled specially (see the postgres/mssql, it exactly prevents any use of alias o the updated table) and the other being the one causing problems here (and needing similar fix).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Confirmed, the query above works against the 4 RDBMS. Also, this quick grep: grep -ri "UPDATE {.*} " * | grep -vi SE Only reveals 3 uses: lib/accesslib.php: $updatesql = "UPDATE {context} ct, {context_temp} temp lib/accesslib.php: $updatesql = "UPDATE {context} ct question/type/numerical/db/upgrade.php: UPDATE {question_numerical_options} qno with the 2 accesslib ones already being handled specially (see the postgres/mssql, it exactly prevents any use of alias o the updated table) and the other being the one causing problems here (and needing similar fix). Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'd suggest to add this to all the branches too, to have it covered and somehow documented:

          https://github.com/stronk7/moodle/compare/master...MDL-30739

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'd suggest to add this to all the branches too, to have it covered and somehow documented: https://github.com/stronk7/moodle/compare/master...MDL-30739 Ciao
          Hide
          Tim Hunt added a comment -

          The reason we have not seen this before is that the upgrade script was only changed recently: MDL-30021.

          Show
          Tim Hunt added a comment - The reason we have not seen this before is that the upgrade script was only changed recently: MDL-30021 .
          Tim Hunt made changes -
          Link This issue is a regression caused by MDL-30021 [ MDL-30021 ]
          Hide
          Tim Hunt added a comment -

          aria pn, are you able to test this proposed fix for us? Thanks.

          I suppose we really need to test this on all supported DBs. How depressing.

          Show
          Tim Hunt added a comment - aria pn, are you able to test this proposed fix for us? Thanks. I suppose we really need to test this on all supported DBs. How depressing.
          Tim Hunt made changes -
          Status Open [ 1 ] Waiting for peer review [ 10012 ]
          Pull Master Diff URL https://github.com/timhunt/moodle/compare/master...MDL-30739
          Pull Master Branch MDL-30739
          Pull 2.0 Diff URL https://github.com/timhunt/moodle/compare/MOODLE_20_STABLE...MDL-30739_20
          Pull from Repository git://github.com/timhunt/moodle.git
          Pull 2.0 Branch MDL-30739_20
          Pull 2.1 Branch MDL-30739_21
          Pull 2.2 Diff URL https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-30739_22
          Pull 2.1 Diff URL https://github.com/timhunt/moodle/compare/MOODLE_21_STABLE...MDL-30739_21
          Pull 2.2 Branch MDL-30739_22
          Tim Hunt made changes -
          Testing Instructions upgrading 1.9.15 to any of 2.* version with mssql_n or sqlsrv db connection We need to do this in all supported databases:

          1) In a Moodle 1.9 site, create a course with a quiz and four questions in its question bank.
          * question a) A numerical question with no units.
          * question b) A numerical question with some units.
          * question c) A calculated question with no units.
          * question d) A calculated question with some units.

          2) Upgrade the site to Moodle 2.0. Make sure the upgrade completes with no errors.

          3) Verify that:
          * question a) is set to Unit handling: "Units are not used at all. Only the numerical value is graded."
          * question b) is set to Unit handling: "Units are optional. If a unit is entered, it is used to convert the reponse to Unit 1 before grading."
          * similarly for c) and d)
          Hide
          Tim Hunt added a comment -

          Actually, 2.0 is not affected. This affects the upgrade from 2.0 to 2.1

          Show
          Tim Hunt added a comment - Actually, 2.0 is not affected. This affects the upgrade from 2.0 to 2.1
          Tim Hunt made changes -
          Pull 2.0 Diff URL https://github.com/timhunt/moodle/compare/MOODLE_20_STABLE...MDL-30739_20
          Pull 2.0 Branch MDL-30739_20
          Testing Instructions We need to do this in all supported databases:

          1) In a Moodle 1.9 site, create a course with a quiz and four questions in its question bank.
          * question a) A numerical question with no units.
          * question b) A numerical question with some units.
          * question c) A calculated question with no units.
          * question d) A calculated question with some units.

          2) Upgrade the site to Moodle 2.0. Make sure the upgrade completes with no errors.

          3) Verify that:
          * question a) is set to Unit handling: "Units are not used at all. Only the numerical value is graded."
          * question b) is set to Unit handling: "Units are optional. If a unit is entered, it is used to convert the reponse to Unit 1 before grading."
          * similarly for c) and d)
          We need to do this in all supported databases:

          1) In a Moodle 1.9 or 2.0 site, create a course with a quiz and four questions in its question bank.
          * question a) A numerical question with no units.
          * question b) A numerical question with some units.
          * question c) A calculated question with no units.
          * question d) A calculated question with some units.

          2) Upgrade the site to Moodle 2.1 or 2.2. Make sure the upgrade completes with no errors.

          3) Verify that:
          * question a) is set to Unit handling: "Units are not used at all. Only the numerical value is graded."
          * question b) is set to Unit handling: "Units are optional. If a unit is entered, it is used to convert the reponse to Unit 1 before grading."
          * similarly for c) and d)
          Affects Version/s 2.0.6 [ 11250 ]
          Hide
          Tim Hunt added a comment -

          To INTEGRATORS please cherry-pick Eloy's new unit tests onto all branches too. Thanks. https://github.com/stronk7/moodle/compare/master...MDL-30739

          Show
          Tim Hunt added a comment - To INTEGRATORS please cherry-pick Eloy's new unit tests onto all branches too. Thanks. https://github.com/stronk7/moodle/compare/master...MDL-30739
          Hide
          aria pn added a comment -

          Yes it works Tim. Thanks.

          Show
          aria pn added a comment - Yes it works Tim. Thanks.
          Hide
          Tim Hunt added a comment -

          Thanks for confirming. Submitting for integration now.

          Show
          Tim Hunt added a comment - Thanks for confirming. Submitting for integration now.
          Tim Hunt made changes -
          Status Waiting for peer review [ 10012 ] Waiting for integration review [ 10010 ]
          Eloy Lafuente (stronk7) made changes -
          Currently in integration Yes [ 10041 ]
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator stronk7
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks! (and test commit too)

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (and test commit too)
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Affects Version/s 2.3 [ 10657 ]
          Fix Version/s 2.1.4 [ 11452 ]
          Fix Version/s 2.2.1 [ 11456 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Hide
          David Mudrak added a comment -

          Let's rock'n'test

          Show
          David Mudrak added a comment - Let's rock'n'test
          David Mudrak made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Tester mudrd8mz
          Hide
          David Mudrak added a comment -

          The upgrade passes on Microsoft SQL Server Express Edition with Advanced Services ver 10.50.1617.0. The the test 3), the d) ended with "The unit must be given..." Could that be or did I do a mistake?

          Show
          David Mudrak added a comment - The upgrade passes on Microsoft SQL Server Express Edition with Advanced Services ver 10.50.1617.0. The the test 3), the d) ended with "The unit must be given..." Could that be or did I do a mistake?
          Hide
          David Mudrak added a comment -

          The test passed on PostgreSQL 9.0.5 with the same result

          Show
          David Mudrak added a comment - The test passed on PostgreSQL 9.0.5 with the same result
          Hide
          David Mudrak added a comment -

          Ah I see. For the d) I actually defined "UNIT GRADED" instead of "UNIT USED".

          Show
          David Mudrak added a comment - Ah I see. For the d) I actually defined "UNIT GRADED" instead of "UNIT USED".
          Hide
          David Mudrak added a comment -

          Finally, the test passed on MySQL 5.1.56

          Show
          David Mudrak added a comment - Finally, the test passed on MySQL 5.1.56
          Hide
          David Mudrak added a comment -

          Passing the test. Upgrade goes through correctly on the main supported DBs and the resulting unit settings are as expected. Thanks for a nice opportunity to play with the quiz, questions, MSSQL and MySQL.

          Show
          David Mudrak added a comment - Passing the test. Upgrade goes through correctly on the main supported DBs and the resulting unit settings are as expected. Thanks for a nice opportunity to play with the quiz, questions, MSSQL and MySQL.
          David Mudrak made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Tim Hunt added a comment -

          Thanks for all the upgrade testing David!

          Show
          Tim Hunt added a comment - Thanks for all the upgrade testing David!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

          Now... disconnect, relax and enjoy the next days, yay!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao
          Eloy Lafuente (stronk7) made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 23/Dec/11

            People

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

              Dates

              • Created:
                Updated:
                Resolved: