Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.8, 1.8.1, 1.8.2
    • Fix Version/s: 1.8.4
    • Component/s: Gradebook, Groups
    • Labels:
      None
    • Database:
      Microsoft SQL
    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE
    • Rank:
      26009

      Description

      Same issue as MDL-9081being that groups do not show in the gradebook running with MSsql database but using same code with mysql groups do work.
      Any Ideas?

        Activity

        Hide
        Warren Gowen added a comment -

        I have traced this to /group/lib/legacylib.php at line 115 and removed the DISTINCT keyword it now works. However I am not sure if this is going to cause any other issues.

        Show
        Warren Gowen added a comment - I have traced this to /group/lib/legacylib.php at line 115 and removed the DISTINCT keyword it now works. However I am not sure if this is going to cause any other issues.
        Hide
        Wen Hao Chuang added a comment -

        Hi Anthony this was assigned to Jeff who is no longer maintaining the GBPv2, could you please take a look at this and see if this got fixed already? Thanks!

        Show
        Wen Hao Chuang added a comment - Hi Anthony this was assigned to Jeff who is no longer maintaining the GBPv2, could you please take a look at this and see if this got fixed already? Thanks!
        Hide
        Anthony Borrow added a comment -

        Warren - My apologies for the slow response to this; however, it seems like you were able to get it working. Have you had any troubles? Not being familiar with MSSQL, it seems to me that this is related to the order by clause in the SQL statement. I'm basing this off of what I read at:

        http://www.codingforums.com/archive/index.php?t-27947.html

        As I looked at the query I'm not sure there is any reason to have the DISTINCT there. AFAIK (actually I justed checked), there is a unique index for groupid, userid so I do not see where the benefit is in making it distinct. The function seems to have been copied from somewhere else and I suspect that it could use one of the main lib functions. In any case, I will check with some of the more experienced developers and see how this might best be resolved. Peace - Anthony

        Show
        Anthony Borrow added a comment - Warren - My apologies for the slow response to this; however, it seems like you were able to get it working. Have you had any troubles? Not being familiar with MSSQL, it seems to me that this is related to the order by clause in the SQL statement. I'm basing this off of what I read at: http://www.codingforums.com/archive/index.php?t-27947.html As I looked at the query I'm not sure there is any reason to have the DISTINCT there. AFAIK (actually I justed checked), there is a unique index for groupid, userid so I do not see where the benefit is in making it distinct. The function seems to have been copied from somewhere else and I suspect that it could use one of the main lib functions. In any case, I will check with some of the more experienced developers and see how this might best be resolved. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Nick - I added you as a watcher since you originally wrote this code. I would very much appreciate your opinion/insights on this. Am I overlooking a scenario whereby we are benefiting from having the DISTINCT here? Just to be clear - I'm referring to the SQL query in the function get_group_users in /group/lib/legacylib.php (around line 115). Thanks for any insights you might be able to provide. Peace - Anthony

        Show
        Anthony Borrow added a comment - Nick - I added you as a watcher since you originally wrote this code. I would very much appreciate your opinion/insights on this. Am I overlooking a scenario whereby we are benefiting from having the DISTINCT here? Just to be clear - I'm referring to the SQL query in the function get_group_users in /group/lib/legacylib.php (around line 115). Thanks for any insights you might be able to provide. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        Yu - I added you as a watcher; however, if you want to take over this issue I would be happy to surrender it to you. Unfortunately, I do not have a MSSQL server to test against but looking back on the code the DISTINCT did not seem to make that much sense. Any thoughts? Peace - Anthony

        Show
        Anthony Borrow added a comment - Yu - I added you as a watcher; however, if you want to take over this issue I would be happy to surrender it to you. Unfortunately, I do not have a MSSQL server to test against but looking back on the code the DISTINCT did not seem to make that much sense. Any thoughts? Peace - Anthony
        Hide
        Yu Zhang added a comment -

        Hi Anthony,

        I had a quick look, I think it's ok to deleted the DISTINCT since the first field is always u.id in all calls to this function (either u.id passed in, or u.* as default). get_records_sql() should force the uniqueness of u.id so I think this can be taken out.

        Cheers,

        Yu

        Show
        Yu Zhang added a comment - Hi Anthony, I had a quick look, I think it's ok to deleted the DISTINCT since the first field is always u.id in all calls to this function (either u.id passed in, or u.* as default). get_records_sql() should force the uniqueness of u.id so I think this can be taken out. Cheers, Yu
        Hide
        Anthony Borrow added a comment -

        Yu - Thanks for taking a look and confirming my suspicion that the DISTINCT is not really doing anything in this particular context. In order to allow for easier MSSQL access I am going to remove it from 18STABLE, 19STABLE, and HEAD. If it produces some sort of unforeseen problem we can put it back in and specifically re-write the query for MSSQL; however, this approach of simply removing the DISTINCT seems to me to be the cleanest. Peace - Anthony

        Show
        Anthony Borrow added a comment - Yu - Thanks for taking a look and confirming my suspicion that the DISTINCT is not really doing anything in this particular context. In order to allow for easier MSSQL access I am going to remove it from 18STABLE, 19STABLE, and HEAD. If it produces some sort of unforeseen problem we can put it back in and specifically re-write the query for MSSQL; however, this approach of simply removing the DISTINCT seems to me to be the cleanest. Peace - Anthony
        Hide
        Anthony Borrow added a comment -

        suggested patch of removing DISTINCT from query

        Show
        Anthony Borrow added a comment - suggested patch of removing DISTINCT from query
        Hide
        Anthony Borrow added a comment -

        OK - I committed the change to 18STABLE; however, the legacylib.php file does not exist in 19STABLE or HEAD. I did check the /lib/grouplib.php file and similar functions were not using the DISTINCT. So I believe this is limited to 18STABLE. I'm not sure how groups were handled in 17STABLE or 16STABLE; however, if a similar issue exists then we can re-open. I'm going to mark this as resolved. Peace - Anthony

        Show
        Anthony Borrow added a comment - OK - I committed the change to 18STABLE; however, the legacylib.php file does not exist in 19STABLE or HEAD. I did check the /lib/grouplib.php file and similar functions were not using the DISTINCT. So I believe this is limited to 18STABLE. I'm not sure how groups were handled in 17STABLE or 16STABLE; however, if a similar issue exists then we can re-open. I'm going to mark this as resolved. Peace - Anthony

          People

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

            Dates

            • Created:
              Updated:
              Resolved: