Moodle
  1. Moodle
  2. MDL-34527

Error when viewing participant's posts page

    Details

    • Database:
      Microsoft SQL
    • Testing Instructions:
      Hide

      Repeat the following for all supported databases, especially Oracle and Mssql!

      1. Log in as admin/teacher
      2. Navigate to a course with students who have posted to the forums
      3. Navigate to Navigation > CourseXXX > Participants
      4. Select a participant
      5. Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Posts
      6. VERIFY: the results are accurate and no errrors are returned
      7. Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Discussions
      8. VERIFY: the results are accurate and no errrors are returned
      Show
      Repeat the following for all supported databases, especially Oracle and Mssql! Log in as admin/teacher Navigate to a course with students who have posted to the forums Navigate to Navigation > CourseXXX > Participants Select a participant Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Posts VERIFY: the results are accurate and no errrors are returned Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Discussions VERIFY: the results are accurate and no errrors are returned
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      wip-MDL-34527-m24
    • Rank:
      42951

      Description

      There is an error when you view the Posts and Discussions pages related to a course participant. This seems to happen in MS SQL and Oracle but not MySQL or PostgreSQL.

      The problem is that we are using the DISTINCT qualifier on all fields from the forum table, which includes a long text field. We need to investigate what fields are actually needed from this query. I started tracing this back, but it will take some time to determine which fields are actually needed.

      Default exception handler: Error reading from database Debug: The ntext data type cannot be selected as DISTINCT because it is not comparable.
      SELECT DISTINCT f.*, cm.id AS cmid
                  FROM mdl_forum f
                  JOIN mdl_course_modules cm ON cm.instance = f.id
                  JOIN mdl_modules m ON m.id = cm.module
                  JOIN mdl_forum_discussions fd ON fd.forum = f.id
                          JOIN mdl_forum_posts fp ON fp.discussion = fd.id
                  WHERE m.name = 'forum' AND f.course = ? AND fp.userid = ?
      [array (
        0 => 2203,
        1 => '33713',
      )]
      * line 394 of \lib\dml\moodle_database.php: dml_read_exception thrown
      * line 255 of \lib\dml\mssql_native_moodle_database.php: call to moodle_database->query_end()
      * line 711 of \lib\dml\mssql_native_moodle_database.php: call to mssql_native_moodle_database->query_end()
      * line 740 of \lib\dml\mssql_native_moodle_database.php: call to mssql_native_moodle_database->get_recordset_sql()
      * line 7925 of \mod\forum\lib.php: call to mssql_native_moodle_database->get_records_sql()
      * line 8078 of \mod\forum\lib.php: call to forum_get_forums_user_posted_in()
      * line 121 of \mod\forum\user.php: call to forum_get_posts_by_user()
      

      Replication steps:

      1. Log in as admin/teacher
      2. Navigate to a course with students who have posted to the forums
      3. Navigate to Navigation > CourseXXX > Participants
      4. Select a participant
      5. Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Posts
      6. Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Discussions

      Expected result: The student's posts are reported

      Actual result: With MS SQL and Oracle, an error is shown

      The offending query is at around 8046 in /mod/forum/lib

        Issue Links

          Activity

          Hide
          Fred Weiss added a comment -

          Changing field mdl_forum intro from ntext to nvarchar(max) seems to let the query run successfully.

          I tried this on my test site.

          Show
          Fred Weiss added a comment - Changing field mdl_forum intro from ntext to nvarchar(max) seems to let the query run successfully. I tried this on my test site.
          Hide
          Michael de Raadt added a comment -

          Hi, Fred.

          Could you please explain the circumstances that led to this error?

          Also, what database are you using?

          Converting a DB field is probably not going to be desirable, but we should be able to modify the query to work around this restriction.

          Show
          Michael de Raadt added a comment - Hi, Fred. Could you please explain the circumstances that led to this error? Also, what database are you using? Converting a DB field is probably not going to be desirable, but we should be able to modify the query to work around this restriction.
          Hide
          Michael de Raadt added a comment -

          Hi, Fred.

          Could you please provide the additional information I asked for earlier?

          Show
          Michael de Raadt added a comment - Hi, Fred. Could you please provide the additional information I asked for earlier?
          Hide
          Michael de Raadt added a comment -

          Sam: You seem to be the last person to touch this. Perhaps you might like to look at it.

          Show
          Michael de Raadt added a comment - Sam: You seem to be the last person to touch this. Perhaps you might like to look at it.
          Hide
          Fred Weiss added a comment -

          Deleted my last post as it was incorrect. Following the replication steps above I got the error message listed above.
          Replication steps:

          Log in as admin/teacher
          Navigate to a course with students who have posted to the forums
          Navigate to Navigation > CourseXXX > Participants
          Select a participant
          Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Posts
          Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Discussions

          I checked in the tables mentioned and found two contained fields with the data type = ntext.

          mdl_forum_posts - column name - message
          mdl_forum - column name - intro

          I changed both of these to type nvarchar(max) and the report works correctly.

          This problem is reported a few other times in tracker and moodle.org forums in other areas. Each time it is recommended to make this change and it rectifies the problem.

          The question was asked why use ntext data type at all. I am not however a DB analyst so can't really comment.

          Show
          Fred Weiss added a comment - Deleted my last post as it was incorrect. Following the replication steps above I got the error message listed above. Replication steps: Log in as admin/teacher Navigate to a course with students who have posted to the forums Navigate to Navigation > CourseXXX > Participants Select a participant Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Posts Navigate to Navigation > CourseXXX > Participants > ParticipantXXX > Forum posts > Discussions I checked in the tables mentioned and found two contained fields with the data type = ntext. mdl_forum_posts - column name - message mdl_forum - column name - intro I changed both of these to type nvarchar(max) and the report works correctly. This problem is reported a few other times in tracker and moodle.org forums in other areas. Each time it is recommended to make this change and it rectifies the problem. The question was asked why use ntext data type at all. I am not however a DB analyst so can't really comment.
          Hide
          Michael de Raadt added a comment -

          I'm bumping this issue as it has been duplicated a number of times.

          It shouldn't be too hard to fix.

          Show
          Michael de Raadt added a comment - I'm bumping this issue as it has been duplicated a number of times. It shouldn't be too hard to fix.
          Hide
          Sam Hemelryk added a comment -

          Ok, I've had a look at this one using Oracle and could reproduce similar issues there.
          I've made a couple of adjustments to the structure of a couple of queries that were breaking under oracle and have pushed them up for peer-review now.

          Unfortunately I don't have MSSQL server at the moment, my windows machine has fritzed itself.
          If the peer-reviewer could please test that, or find someone in the office to that would be most appreciated.
          It is an easy test, just follow the replication instructions.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Ok, I've had a look at this one using Oracle and could reproduce similar issues there. I've made a couple of adjustments to the structure of a couple of queries that were breaking under oracle and have pushed them up for peer-review now. Unfortunately I don't have MSSQL server at the moment, my windows machine has fritzed itself. If the peer-reviewer could please test that, or find someone in the office to that would be most appreciated. It is an easy test, just follow the replication instructions. Many thanks Sam
          Hide
          Ankit Agarwal added a comment -

          Sorry Sam,
          I didnt realize, it needed MSSQL. I have unassigned myself from the reviewer.
          A couple of things that I noticed are:-

          1. Why do we need a Distinct in existing query? should it be returning distinct results anyway?
          2. Will it be better not to reuse the shortname "f" for forum table?

          Thanks

          Show
          Ankit Agarwal added a comment - Sorry Sam, I didnt realize, it needed MSSQL. I have unassigned myself from the reviewer. A couple of things that I noticed are:- Why do we need a Distinct in existing query? should it be returning distinct results anyway? Will it be better not to reuse the shortname "f" for forum table? Thanks
          Hide
          Sam Hemelryk added a comment -

          Hi Ankit,

          No probs I'll put it back up for peer-review now.
          Regarding the 2 points:

          1. DISTINCT was needed because we were selecting based upon either a discussion or a post but we were really only interested in the forum. By using distinct we could be sure the forum was returned once no matter how many discussions the user had started, or how many posts they had made.
            The new query avoids it by building that as a sub-select using group by and then joining to the sub select.
          2. Reusing a table name within the sub-select doesn't have any adverse consequences that I am aware of. We do it in several statements I think so hopefully it is fine. There's no preference to it that I am aware of either.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Ankit, No probs I'll put it back up for peer-review now. Regarding the 2 points: DISTINCT was needed because we were selecting based upon either a discussion or a post but we were really only interested in the forum. By using distinct we could be sure the forum was returned once no matter how many discussions the user had started, or how many posts they had made. The new query avoids it by building that as a sub-select using group by and then joining to the sub select. Reusing a table name within the sub-select doesn't have any adverse consequences that I am aware of. We do it in several statements I think so hopefully it is fine. There's no preference to it that I am aware of either. Many thanks Sam
          Hide
          Dan Poltawski added a comment -

          Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..

          Show
          Dan Poltawski added a comment - Sending all 'waiting for peer review' issues to integration before freeze, as agreed in Integrators Meeting 19/10/12. We are doing this to ensure any 'integratable issues' will not got missed before freeze..
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          I tested in mssql and its looking good.

          Show
          Dan Poltawski added a comment - I tested in mssql and its looking good.
          Hide
          Dan Poltawski added a comment -

          Thanks Sam, i've integrated this now. I added one commit to fix trailing whitespace.

          Show
          Dan Poltawski added a comment - Thanks Sam, i've integrated this now. I added one commit to fix trailing whitespace.
          Hide
          Michael de Raadt added a comment -

          Test result: Success!

          Tested with MySQL, PostgreSQL, Oracle and MS SQL under master.

          Also tested in MS SQL under 2.2 and 2.3.

          Show
          Michael de Raadt added a comment - Test result: Success! Tested with MySQL, PostgreSQL, Oracle and MS SQL under master. Also tested in MS SQL under 2.2 and 2.3.
          Hide
          Luis de Vasconcelos added a comment -

          Michael, for MSSQL are you using SQLSRV or FreeTDS?

          Show
          Luis de Vasconcelos added a comment - Michael, for MSSQL are you using SQLSRV or FreeTDS?
          Hide
          Dan Poltawski added a comment -

          (I was testing MSSQL in freedts, for the records)

          Show
          Dan Poltawski added a comment - (I was testing MSSQL in freedts, for the records)
          Hide
          Dan Poltawski added a comment -

          Note that I don't think it should mattter, Luis, it was the query executed on the server which was the problem.

          Show
          Dan Poltawski added a comment - Note that I don't think it should mattter, Luis, it was the query executed on the server which was the problem.
          Hide
          Michael de Raadt added a comment -

          I tested this natively with SQL Server, so I think we have this covered.

          Show
          Michael de Raadt added a comment - I tested this natively with SQL Server, so I think we have this covered.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!
          Hide
          Leo Furze-Waddock added a comment -

          Similar issue discovered in Forum Posts

          Show
          Leo Furze-Waddock added a comment - Similar issue discovered in Forum Posts

            People

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

              Dates

              • Created:
                Updated:
                Resolved: