Uploaded image for project: '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
    • Status: Closed
    • Priority: 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:

      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()

        Gliffy Diagrams

          Issue Links

            Activity

            ariapn aria pn created issue -
            salvetore 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 ]
            salvetore Michael de Raadt made changes -
            Assignee Rajesh Taneja [ rajeshtaneja ] Tim Hunt [ timhunt ]
            Hide
            timhunt 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
            timhunt 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?
            timhunt Tim Hunt made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Labels triaged
            Hide
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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
            timhunt 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
            timhunt Tim Hunt added a comment - The reason we have not seen this before is that the upgrade script was only changed recently: MDL-30021 .
            timhunt Tim Hunt made changes -
            Link This issue is a regression caused by MDL-30021 [ MDL-30021 ]
            Hide
            timhunt 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
            timhunt 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.
            timhunt 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
            timhunt 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
            timhunt Tim Hunt added a comment -

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

            Show
            timhunt Tim Hunt added a comment - Actually, 2.0 is not affected. This affects the upgrade from 2.0 to 2.1
            timhunt 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
            timhunt 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
            timhunt 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
            ariapn aria pn added a comment -

            Yes it works Tim. Thanks.

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

            Thanks for confirming. Submitting for integration now.

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

            Integrated, thanks! (and test commit too)

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (and test commit too)
            stronk7 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
            mudrd8mz David Mudrak added a comment -

            Let's rock'n'test

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

            The test passed on PostgreSQL 9.0.5 with the same result

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

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

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

            Finally, the test passed on MySQL 5.1.56

            Show
            mudrd8mz David Mudrak added a comment - Finally, the test passed on MySQL 5.1.56
            Hide
            mudrd8mz 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
            mudrd8mz 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.
            mudrd8mz David Mudrak made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            timhunt Tim Hunt added a comment -

            Thanks for all the upgrade testing David!

            Show
            timhunt Tim Hunt added a comment - Thanks for all the upgrade testing David!
            Hide
            stronk7 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
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  9/Jan/12