Moodle

Please supply info re. correct use of course sortorder field during course insertions/deletions.

Details

  • Type: Task Task
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Not a bug
  • Affects Version/s: 1.6.3, 1.7
  • Fix Version/s: 1.7.5
  • Component/s: Course
  • Labels:
    None
  • Environment:
    Moodle 1.7, PHP 5.1.4, PostgreSQL 8.1.4 on RedHat Linux.
  • Database:
    PostgreSQL
  • Affected Branches:
    MOODLE_16_STABLE, MOODLE_17_STABLE
  • Fixed Branches:
    MOODLE_17_STABLE

Description

We have developed a web page which can be used to create or delete courses. (It can be used simultaneously by multiple users).

The web page uses a single database transaction each time it runs. The transaction can consist of an insert of a single row to the course table, a deletion of a single row from the course table, or an insert and a delete within the same transaction. Note that a delete and insert of a course always applies to the same course within the same category - it does not support category changes at the moment.

This web page periodically fails with duplicate key errors on the course table, field "sortorder". In order to try to prevent this, we call "fix_course_sortorder" in the following places, (but it does not seem to prevent the problem):

1. If the transaction fails;

fix_course_sortorder($category);
fix_course_sortorder();

where $category is the ID of the category the course was being inserted into or deleted from.

2. After a course deletion:

fix_course_sortorder(); (without a category ID passed).

3. Before a course insertion

fix_course_sortorder($category);

(where $category is the ID of the category the course is going to be inserted into)

We also default the sortorder of the new course to the min(sortorder) for courses in that category, minus 1.
If there are no pre-existing courses in that category we default it to 100.

4. After a successful course insertion

fix_course_sortorder(); (without a category ID passed).

Could someone please advise on what the correct approach is to calling fix_course_sortorder around course insertions and deletions, as we appear to be doing something wrong with our solution which is causing the duplicate key conflicts.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Gareth Morgan added a comment -

Martin,

I've been doing a bit of experimentation, and got the following results:

i. if sortorder is negative on all courses within one category, calling fix_course_sortorder without any arguments tends to fix it.

ii. if sortorder is negative on some but not all courses within one category, calling fix_course_sortorder without a category passed does not fix it, but calling it and passing it the category ID that is affected does.

I am going to test some code that will run before the attempt to insert a course row is made, and that does a select distinct on category from the course table where sortorder < 0, and then calls fix_course_sortorder, passing it the category ID. It may avert the problem.

Gareth.

Show
Gareth Morgan added a comment - Martin, I've been doing a bit of experimentation, and got the following results: i. if sortorder is negative on all courses within one category, calling fix_course_sortorder without any arguments tends to fix it. ii. if sortorder is negative on some but not all courses within one category, calling fix_course_sortorder without a category passed does not fix it, but calling it and passing it the category ID that is affected does. I am going to test some code that will run before the attempt to insert a course row is made, and that does a select distinct on category from the course table where sortorder < 0, and then calls fix_course_sortorder, passing it the category ID. It may avert the problem. Gareth.
Hide
Permalink
Martin Dougiamas added a comment -

Hi, Gareth.

Thanks for posting this. Did you come to any conclusions here? Anything we can do in core?

Show
Martin Dougiamas added a comment - Hi, Gareth. Thanks for posting this. Did you come to any conclusions here? Anything we can do in core?
Hide
Permalink
Gareth Morgan added a comment -

Hi, Martin,

I have included code in my program to detect categories with one or more negative values in course sortorders. It then calls fix_course_sortorder one or more times, once for each affected category, passing the category ID to fix_course_sortorder. It then calls fix_course_sortorder once without a category ID argument, at the end.

The code is called before the web service inserts or deletes the course.

global $CFG;
$sqlstring=
"SELECT distinct(category), '1' FROM {$CFG->prefix}course WHERE sortorder < 0";

if($categories = get_records_sql($sqlstring)){

foreach ($categories as $category=>$record) { fix_course_sortorder($category); }

}

//Now call it without any categories specified.
fix_course_sortorder();

There are other calls to fix_course_sortorder following an insertion or deletion and in the event of an error, as detailed in my original bug report.

It appears to be working reliably so far, but I feel that it may perhaps be an over-engineered solution for dealing with this problem.

Thanks,

Gareth.

Show
Gareth Morgan added a comment - Hi, Martin, I have included code in my program to detect categories with one or more negative values in course sortorders. It then calls fix_course_sortorder one or more times, once for each affected category, passing the category ID to fix_course_sortorder. It then calls fix_course_sortorder once without a category ID argument, at the end. The code is called before the web service inserts or deletes the course. global $CFG; $sqlstring= "SELECT distinct(category), '1' FROM {$CFG->prefix}course WHERE sortorder < 0"; if($categories = get_records_sql($sqlstring)){ foreach ($categories as $category=>$record) { fix_course_sortorder($category); } } //Now call it without any categories specified. fix_course_sortorder(); There are other calls to fix_course_sortorder following an insertion or deletion and in the event of an error, as detailed in my original bug report. It appears to be working reliably so far, but I feel that it may perhaps be an over-engineered solution for dealing with this problem. Thanks, Gareth.
Hide
Permalink
Martín Langhoff added a comment -

Hello Gareth,

I'm to blame for both the sortorder mess and for a few other functions in Moodle that perform mass-creation of courses (for some enrolment plugins) and have had to resolve the same problem. Here's my recipe...

fix_course_sortorder() leaves large gaps (of 1000) between categories to make it easy/cheap to add new courses and not have to renumber everything. Of course, if you fill the gap by creating more than 1000, you collide with the next category :-/

The strategy I use is to

  • run fix_course_sortorder()
  • figure out the largest sortorder in the category
  • figure out the gap size with something like select min(sortorder) from mdl_course where sortorder > $mymaxsortorder
  • create they courses 'at the end' of the category by doing $courseorder++
  • if $courseorder gets close to the end of the gap (give yourself some 100 entries in case someone else has been creating courses too), then I run fix_course_sortorder() which will space them out again

Note that I call fix_course_sortorder() with no parameters, which means that it will work across all categories, and will space them out. If you call it with a category, it will work on that category alone, and what you want it to do is space the existing categories out.

Does that strategy make sense? It should also be the fastest – try as much as you want, if you have thousands of courses fix_course_sortorder() is expensive to run so I always try to minimise the number of times it's called.

Show
Martín Langhoff added a comment - Hello Gareth, I'm to blame for both the sortorder mess and for a few other functions in Moodle that perform mass-creation of courses (for some enrolment plugins) and have had to resolve the same problem. Here's my recipe... fix_course_sortorder() leaves large gaps (of 1000) between categories to make it easy/cheap to add new courses and not have to renumber everything. Of course, if you fill the gap by creating more than 1000, you collide with the next category :-/ The strategy I use is to
  • run fix_course_sortorder()
  • figure out the largest sortorder in the category
  • figure out the gap size with something like select min(sortorder) from mdl_course where sortorder > $mymaxsortorder
  • create they courses 'at the end' of the category by doing $courseorder++
  • if $courseorder gets close to the end of the gap (give yourself some 100 entries in case someone else has been creating courses too), then I run fix_course_sortorder() which will space them out again
Note that I call fix_course_sortorder() with no parameters, which means that it will work across all categories, and will space them out. If you call it with a category, it will work on that category alone, and what you want it to do is space the existing categories out. Does that strategy make sense? It should also be the fastest – try as much as you want, if you have thousands of courses fix_course_sortorder() is expensive to run so I always try to minimise the number of times it's called.
Hide
Permalink
Martín Langhoff added a comment -

BTW, you can often cheat and assume the gap is at least 800

Show
Martín Langhoff added a comment - BTW, you can often cheat and assume the gap is at least 800
Hide
Permalink
Jenny Gray added a comment -

I hope no-one minds if I close this off - doing a bit of tidying up. We haven't had any problems with sort order for ages, so I don't think there's a real issue to resolve here in core.

Show
Jenny Gray added a comment - I hope no-one minds if I close this off - doing a bit of tidying up. We haven't had any problems with sort order for ages, so I don't think there's a real issue to resolve here in core.

People

Vote (0)
Watch (2)

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.