Moodle

Fixes for upgrade issues (from 1.6 to 1.8 on Postgres 8.1)

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.8.4, 1.9
  • Fix Version/s: 1.6.6, 1.7.4, 1.8.4, 1.9, 2.0
  • Component/s: Database SQL/XMLDB
  • Labels:
    None
  • Environment:
    Linux 2.6.22, Ubuntu 7.10, Postgres 8.1
  • Database:
    PostgreSQL
  • Affected Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_16_STABLE, MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE, MOODLE_20_STABLE

Description

While doing an upgrade from 1.6.? to MOODLE_18_STABLE for a client, I had to remove a few calls to rebuild_course_cache() which were interfering with the upgrade process:

  • 16_to_18_upgrade_on_postgres.patch

Then, I compared the database schemas of a fresh MOODLE_18_STABLE install with the upgraded database and fixed the following discrepancies:

  • mod-exercise_add_late_field.patch: add the 'late' field which was missing from exercise_submissions
  • mod-scorm_version_field.patch: add the 'version' field which was missing from scorm
  • question-type-rqp_missing_servers_table.patch: adds the missing 'question_rqp_servers' table

These patches are against:

  • MOODLE_18_STABLE
  • MOODLE_19_STABLE
  • CVSHEAD

except for the rqp question type one since it's no longer part of the core. I have committed a fix for that one in contrib HEAD.

After making these changes, I was able to upgrade this install of Moodle 1.6 to 1.8 without any errors or differences in the schema.

Francois

Attachments

  1. HTML File
    16_to_18_upgrade_log.html
    09/Nov/07 12:41 PM
    711 kB
    Francois Marier
  2. Text File
    16_to_18_upgrade_on_postgres_16STABLE.patch
    09/Nov/07 1:11 PM
    0.0 kB
    Francois Marier
  3. Text File
    16_to_18_upgrade_on_postgres_16STABLE.patch
    07/Nov/07 1:35 PM
    1 kB
    Francois Marier
  4. Text File
    16_to_18_upgrade_on_postgres_17STABLE.patch
    09/Nov/07 1:11 PM
    0.4 kB
    Francois Marier
  5. Text File
    16_to_18_upgrade_on_postgres_17STABLE.patch
    07/Nov/07 1:35 PM
    2 kB
    Francois Marier
  6. Text File
    16_to_18_upgrade_on_postgres_18STABLE.patch
    09/Nov/07 1:11 PM
    0.4 kB
    Francois Marier
  7. Text File
    16_to_18_upgrade_on_postgres_18STABLE.patch
    05/Nov/07 1:48 PM
    2 kB
    Francois Marier
  8. Text File
    16_to_18_upgrade_on_postgres_19STABLE.patch
    09/Nov/07 1:12 PM
    0.4 kB
    Francois Marier
  9. Text File
    16_to_18_upgrade_on_postgres_19STABLE.patch
    05/Nov/07 1:48 PM
    2 kB
    Francois Marier
  10. Text File
    16_to_18_upgrade_on_postgres_CVSHEAD.patch
    09/Nov/07 1:12 PM
    0.4 kB
    Francois Marier
  11. Text File
    16_to_18_upgrade_on_postgres_CVSHEAD.patch
    05/Nov/07 1:48 PM
    2 kB
    Francois Marier
  12. Text File
    mod-exercise_add_late_field_16STABLE.patch
    07/Nov/07 1:35 PM
    2 kB
    Francois Marier
  13. Text File
    mod-exercise_add_late_field_17STABLE.patch
    07/Nov/07 1:36 PM
    2 kB
    Francois Marier
  14. Text File
    mod-exercise_add_late_field_18STABLE.patch
    09/Nov/07 8:30 AM
    2 kB
    Francois Marier
  15. Text File
    mod-exercise_add_late_field_18STABLE.patch
    05/Nov/07 1:49 PM
    2 kB
    Francois Marier
  16. Text File
    mod-exercise_add_late_field_19STABLE.patch
    09/Nov/07 8:30 AM
    2 kB
    Francois Marier
  17. Text File
    mod-exercise_add_late_field_19STABLE.patch
    07/Nov/07 1:46 PM
    2 kB
    Francois Marier
  18. Text File
    mod-exercise_add_late_field_19STABLE.patch
    05/Nov/07 1:49 PM
    2 kB
    Francois Marier
  19. Text File
    mod-exercise_add_late_field_CVSHEAD.patch
    09/Nov/07 8:30 AM
    2 kB
    Francois Marier
  20. Text File
    mod-exercise_add_late_field_CVSHEAD.patch
    05/Nov/07 1:49 PM
    2 kB
    Francois Marier
  21. Text File
    mod-scorm_version_field_16STABLE.patch
    09/Nov/07 7:12 AM
    0.5 kB
    Francois Marier
  22. Text File
    mod-scorm_version_field_16STABLE.patch
    07/Nov/07 1:36 PM
    0.5 kB
    Francois Marier
  23. Text File
    mod-scorm_version_field_18STABLE.patch
    05/Nov/07 1:49 PM
    1 kB
    Francois Marier
  24. Text File
    mod-scorm_version_field_19STABLE.patch
    07/Nov/07 1:46 PM
    1 kB
    Francois Marier
  25. Text File
    mod-scorm_version_field_19STABLE.patch
    05/Nov/07 1:49 PM
    1 kB
    Francois Marier
  26. Text File
    mod-scorm_version_field_CVSHEAD.patch
    07/Nov/07 1:46 PM
    1 kB
    Francois Marier
  27. Text File
    mod-scorm_version_field_CVSHEAD.patch
    05/Nov/07 1:49 PM
    1 kB
    Francois Marier
  28. Text File
    question-type-rqp_missing_servers_table_16STABLE.patch
    07/Nov/07 1:36 PM
    2 kB
    Francois Marier
  29. Text File
    question-type-rqp_missing_servers_table_17STABLE.patch
    07/Nov/07 1:37 PM
    2 kB
    Francois Marier
  30. Text File
    question-type-rqp_missing_servers_table_18STABLE.patch
    05/Nov/07 1:50 PM
    2 kB
    Francois Marier

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Francois Marier added a comment -

This should probably not be tagged as "Major". Sorry about that.

Show
Francois Marier added a comment - This should probably not be tagged as "Major". Sorry about that.
Hide
Permalink
Francois Marier added a comment -

To answer your question from Skype:

> Eloy Lafuente: Does Postgres support "backticks" ? Just saw some in the very first lines of
> mod/exercise/lib/postgres.php when reviewing http://tracker.moodle.org/browse/MDL-12023

I've run the following test on Postgres 8.1:

<?php
require_once('config.php');
require_login();
execute_sql(" ALTER TABLE `{$CFG->prefix}exercise_submissions` ADD `late` TINYINT(3) UNSIGNED NOT NULL DEFAULT '0'");
print "\nLast DB error: ".$db->ErrorMsg()."\n";
?>

and got:

Last DB error: ERROR: syntax error at or near "`" LINE 1: ALTER TABLE `mdl_exercise_submissions` ADD `late` TINYINT(3)... ^

Which is likely the reason it had been removed from the branch I am using here at Catalyst.

Show
Francois Marier added a comment - To answer your question from Skype: > Eloy Lafuente: Does Postgres support "backticks" ? Just saw some in the very first lines of > mod/exercise/lib/postgres.php when reviewing http://tracker.moodle.org/browse/MDL-12023 I've run the following test on Postgres 8.1: <?php require_once('config.php'); require_login(); execute_sql(" ALTER TABLE `{$CFG->prefix}exercise_submissions` ADD `late` TINYINT(3) UNSIGNED NOT NULL DEFAULT '0'"); print "\nLast DB error: ".$db->ErrorMsg()."\n"; ?> and got: Last DB error: ERROR: syntax error at or near "`" LINE 1: ALTER TABLE `mdl_exercise_submissions` ADD `late` TINYINT(3)... ^ Which is likely the reason it had been removed from the branch I am using here at Catalyst.
Hide
Permalink
Francois Marier added a comment -

New versions of these patches which keep the module versions in sync between 1.9 and HEAD

Show
Francois Marier added a comment - New versions of these patches which keep the module versions in sync between 1.9 and HEAD
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Oki. About patches 1-5 some questions:

1) Why do you take out some rebuild_course_cache() calls? Is that really necessary (I can see the point about not having too much sense to execute them repeatedly if such data isn't needed by the upgrade script itself). Have you left at least one call enabled?

2) In the 18STABLE version.... there is one "extra" change about one create table statement. Why? If has sense, shouldn't it be in all versions?

Will review the next, in some hours... thanks!

Show
Eloy Lafuente (stronk7) added a comment - Oki. About patches 1-5 some questions: 1) Why do you take out some rebuild_course_cache() calls? Is that really necessary (I can see the point about not having too much sense to execute them repeatedly if such data isn't needed by the upgrade script itself). Have you left at least one call enabled? 2) In the 18STABLE version.... there is one "extra" change about one create table statement. Why? If has sense, shouldn't it be in all versions? Will review the next, in some hours... thanks!
Hide
Permalink
Francois Marier added a comment -

Thanks for reviewing these ones Eloy, I wasn't sure exactly about the two things that you pointed out and was hoping to discuss them a bit here

I made these changes in order to be able to go through the upgrade successfully. I don't remember exactly what the errors were but I think it had something to do with role assignments being done without the role tables being there yet.

I will do another test upgrade to see if I can get the exact error message for you.

Show
Francois Marier added a comment - Thanks for reviewing these ones Eloy, I wasn't sure exactly about the two things that you pointed out and was hoping to discuss them a bit here I made these changes in order to be able to go through the upgrade successfully. I don't remember exactly what the errors were but I think it had something to do with role assignments being done without the role tables being there yet. I will do another test upgrade to see if I can get the exact error message for you.
Hide
Permalink
Francois Marier added a comment -

By the way, I'll commit the postgres DB fixes to exercise, scorm and rqp today once Penny has reviewed them.

Show
Francois Marier added a comment - By the way, I'll commit the postgres DB fixes to exercise, scorm and rqp today once Penny has reviewed them.
Hide
Permalink
Francois Marier added a comment -

Updated patch for mod/scorm on 1.6.

Note that on 1.6 and 1.7, the postgres upgrade script is fine.

Show
Francois Marier added a comment - Updated patch for mod/scorm on 1.6. Note that on 1.6 and 1.7, the postgres upgrade script is fine.
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Great no problem at all!

Just be careful with version files (I think it's easy to move the versions to the same value because there weren't more changes between them). I've quick-reviewed the DB changes (attachments nº6 and so on) and look correct.

Just have some doubts about the 2 points exposed above (the need to keep at least ONE rebuild_course_cache() call and the create table change).

Anyway, I assign this bug to you... (I'll remain as watcher).

Happy commits with Penny (and don't forget tagging!)

Great work, thanks! Ciao

Show
Eloy Lafuente (stronk7) added a comment - Great no problem at all! Just be careful with version files (I think it's easy to move the versions to the same value because there weren't more changes between them). I've quick-reviewed the DB changes (attachments nº6 and so on) and look correct. Just have some doubts about the 2 points exposed above (the need to keep at least ONE rebuild_course_cache() call and the create table change). Anyway, I assign this bug to you... (I'll remain as watcher). Happy commits with Penny (and don't forget tagging!) Great work, thanks! Ciao
Hide
Permalink
Francois Marier added a comment -

Get rid of backticks in the Postgres upgrade file

Show
Francois Marier added a comment - Get rid of backticks in the Postgres upgrade file
Hide
Permalink
Francois Marier added a comment -

All patches except 1-5 were committed with Penny.

Show
Francois Marier added a comment - All patches except 1-5 were committed with Penny.
Hide
Permalink
Francois Marier added a comment -

Alright, after doing more tests on this Postgres upgrade from 1.6 to 1.8, it turns out that the only necessary change in those 16_to_18_upgrade patches is the removal of "rebuild_course_cache()" in lib/db/upgrade.php.

How should we fix this?

Show
Francois Marier added a comment - Alright, after doing more tests on this Postgres upgrade from 1.6 to 1.8, it turns out that the only necessary change in those 16_to_18_upgrade patches is the removal of "rebuild_course_cache()" in lib/db/upgrade.php. How should we fix this?
Hide
Permalink
Petr Škoda (skodak) added a comment -

Adding Tim, ML says it is the bug related to quiz commit you asked about today...

Show
Petr Škoda (skodak) added a comment - Adding Tim, ML says it is the bug related to quiz commit you asked about today...
Hide
Permalink
Martin Dougiamas added a comment -

No no no no no Francois you can't make database changes in the stable branches!

http://moodle.cvs.sourceforge.net/moodle/moodle/mod/scorm/db/upgrade.php

Have you done this anywhere else?

Show
Martin Dougiamas added a comment - No no no no no Francois you can't make database changes in the stable branches! http://moodle.cvs.sourceforge.net/moodle/moodle/mod/scorm/db/upgrade.php Have you done this anywhere else?
Hide
Permalink
Martin Dougiamas added a comment -

My apologies, I over reacted there. Generally you should not, but I agree that in this case it is OK and works fine.

Show
Martin Dougiamas added a comment - My apologies, I over reacted there. Generally you should not, but I agree that in this case it is OK and works fine.
Hide
Permalink
Francois Marier added a comment -

The rebuild_course_cache() errors have been moved to MDL-12160

Show
Francois Marier added a comment - The rebuild_course_cache() errors have been moved to MDL-12160

People

Vote (1)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.