Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Critical
-
Resolution: Fixed
-
Affects Version/s: 2.2.4
-
Fix Version/s: 2.2.6, 2.3.3, STABLE backlog
-
Component/s: Forum
-
Labels:
-
Database:Microsoft SQL
-
Testing Instructions:
-
Affected Branches:MOODLE_22_STABLE
-
Fixed Branches:MOODLE_22_STABLE, MOODLE_23_STABLE
-
Pull from Repository:
-
Pull Master Branch:wip-
MDL-34527-m24 -
Pull Master Diff URL:
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:
- 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
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
Gliffy Diagrams
Attachments
Issue Links
- has been marked as being related by
-
-
- Closed
-
-
-
- Closed
-
- is duplicated by
-
-
- Closed
-
-
-
- Closed
-
-
-
- Closed
-
- will help resolve
-
-
- Open
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Links Hierarchy
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.
Hi, Fred.
Could you please provide the additional information I asked for earlier?
Sam: You seem to be the last person to touch this. Perhaps you might like to look at it.
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.
I'm bumping this issue as it has been duplicated a number of times.
It shouldn't be too hard to fix.
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
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
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
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..
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 
I tested in mssql and its looking good.
Thanks Sam, i've integrated this now. I added one commit to fix trailing whitespace.
Test result: 
Success!
Tested with MySQL, PostgreSQL, Oracle and MS SQL under master.
Also tested in MS SQL under 2.2 and 2.3.
Michael, for MSSQL are you using SQLSRV or FreeTDS?
(I was testing MSSQL in freedts, for the records)
Note that I don't think it should mattter, Luis, it was the query executed on the server which was the problem.
I tested this natively with SQL Server, so I think we have this covered.
Amazed. Inspired. Grateful. That’s how your generosity makes me feel.
(not really)
Closing, thanks!
Similar issue discovered in Forum Posts
Changing field mdl_forum intro from ntext to nvarchar(max) seems to let the query run successfully.
I tried this on my test site.