Support for Microsoft SQL Server

[mrkskwsnck]

Unfortunately, the course statistics do not seem to work at all but lead to an SQL Server error. Anyhow, we would like to give it a try.

Maybe the most important fact is that our Moodle platform runs with Microsoft SQL Server as database.

System

• Moodle 3.9.4+ (Build: 20210226)
• DB: Microsoft SQL Server
• Plug-ins:
  • Learning Analytics Log v0.8.0 (Build: 2020102200)
  • Learning Analytics v0.16.0 (Build: 2021021500)

Steps to reproduce

• Set up Moodle with SQL Server
• Install plug-ins Learning Analytics Log and Learning Analytics
• Enable log store
• Navigate to the course statistics of an arbitrary course (e.g. https://moodle.example.org/local/learning_analytics/index.php/reports/coursedashboard?course=42)
• The error message appears immediately

Screenshot

Full error message

Debug info: SQLState: 42S22<br>
Error Code: 207<br>
Message: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Invalid column name 'week'.<br>

SELECT (FLOOR((l.timecreated - 1604271600) / (7 * 60 * 60 * 24)) + 1)
AS WEEK,
COUNT(*) clicks
FROM mdl_logstore_lanalytics_log l
WHERE l.courseid = '26'
GROUP BY week
ORDER BY week;
[array (
0 => 26,
)]
Error code: dmlreadexception
Stack trace:

    line 486 of /lib/dml/moodle_database.php: dml_read_exception thrown
    line 324 of /lib/dml/sqlsrv_native_moodle_database.php: call to moodle_database->query_end()
    line 431 of /lib/dml/sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->query_end()
    line 900 of /lib/dml/sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->do_query()
    line 976 of /lib/dml/sqlsrv_native_moodle_database.php: call to sqlsrv_native_moodle_database->get_recordset_sql()
    line 63 of /local/learning_analytics/reports/coursedashboard/classes/query_helper.php: call to sqlsrv_native_moodle_database->get_records_sql()
    line 50 of /local/learning_analytics/reports/coursedashboard/lareport_coursedashboard.php: call to lareport_coursedashboard\query_helper::query_weekly_activity()
    line 460 of /local/learning_analytics/reports/coursedashboard/lareport_coursedashboard.php: call to lareport_coursedashboard->activiyoverweeks()
    line 46 of /local/learning_analytics/classes/router.php: call to lareport_coursedashboard->run()
    line 103 of /local/learning_analytics/classes/router.php: call to local_learning_analytics\router::run_report_or_page()
    line 118 of /local/learning_analytics/index.php: call to local_learning_analytics\router::run()

[thomasdondorf]

We are aware of that issue as we are currently only testing with MySQL/MariaDB/Postgres. That's why MySQL/MariaDB/Postgres is listed as requirement in the documentation.

Maybe we implement this in the future at some point, but it's not on our task list currently. But we are happy to accept any pull requests implementing other database systems.

[mrkskwsnck]

I hacked around a little and found a solution, that works for me :-) However, this is nothing I would fill a PR with.

TL;TR T-SQL (which is SQL Server′s syntax) cannot GROUP BY column aliases

diff --git a/reports/coursedashboard/classes/query_helper.php b/reports/coursedashboard/classes/query_helper.php
index 8fcdb3c..a3b36e6 100644
--- a/reports/coursedashboard/classes/query_helper.php
+++ b/reports/coursedashboard/classes/query_helper.php
@@ -56,7 +56,7 @@ class query_helper {
         COUNT(*) clicks
         FROM {logstore_lanalytics_log} l
         WHERE l.courseid = ?
-        GROUP BY week
+        GROUP BY (FLOOR((l.timecreated - {$mondaytimestamp}) / (7 * 60 * 60 * 24)) + 1)
         ORDER BY week;
 SQL;
 
@@ -84,7 +84,7 @@ SQL;
             ON e.id = ue.enrolid
         WHERE u.deleted = 0
             AND e.courseid = ?
-        GROUP BY time
+        GROUP BY ue.timestart
         ORDER BY time;
 SQL;
 
@@ -143,7 +143,7 @@ SQL;
         $oneweekago = $date->getTimestamp();
 
         $query = <<<SQL
-        SELECT
+        SELECT TOP 3
             cm.id AS cmid,
             m.name as modname,
             COUNT(*) AS hits
@@ -161,7 +161,6 @@ SQL;
         GROUP BY cm.id, m.name
         HAVING count(*) >= ?
         ORDER BY hits DESC
-        LIMIT 3
 SQL;
 
         return $DB->get_records_sql($query, [$courseid, $oneweekago, max(1, $privacythreshold)]);

Now, I am asking for further advice on how to differentiate which DBMS is present at runtime. Any suggestions?

if (DBMS == SQL_SERVER) {
    # Take my hack
} else {
    # Do it as usual
}

[thomasdondorf]

@mrkskwsnck That's great. If you are willing to test, I can make a SQL Server branch in the next days and then you'll give it a try.

May I ask where are you planning on using it? :)

[mrkskwsnck]

I updated my patch for SQL Server (MSSQL) to match version 0.18.0 of the plug-in.

Anyhow, beware of statistic errors, because my focus was to make the SQL statements compile and run. For example I am not sure about my conversion of the function FROM_UNIXTIME() to the SQL Servers′s DATEPART() equivalent. It seems MySQL/MariaDB count weekdays from sunday to saturday as 0–6 but SQL Server counts 1–7 😩

diff --git a/reports/activities/classes/query_helper.php b/reports/activities/classes/query_helper.php
index 5d2165c..5921c15 100644
--- a/reports/activities/classes/query_helper.php
+++ b/reports/activities/classes/query_helper.php
@@ -76,7 +76,7 @@ SQL;
         $oneweekago = $date->getTimestamp();
 
         $query = <<<SQL
-        SELECT
+        SELECT TOP 3
             cm.id AS cmid,
             m.name as modname,
             COUNT(*) AS hits
@@ -94,7 +94,6 @@ SQL;
         GROUP BY cm.id, m.name
         HAVING count(*) >= ?
         ORDER BY hits DESC
-        LIMIT 3
 SQL;
 
         return $DB->get_records_sql($query, [$courseid, $oneweekago, max(1, $privacythreshold)]);
diff --git a/reports/coursedashboard/classes/query_helper.php b/reports/coursedashboard/classes/query_helper.php
index c82ef0b..3d588a4 100644
--- a/reports/coursedashboard/classes/query_helper.php
+++ b/reports/coursedashboard/classes/query_helper.php
@@ -45,7 +45,7 @@ class query_helper {
         COUNT(*) clicks
         FROM {logstore_lanalytics_log} l
         WHERE l.courseid = ?
-        GROUP BY week
+        GROUP BY (FLOOR((l.timecreated - {$mondaytimestamp}) / (7 * 60 * 60 * 24)) + 1)
         ORDER BY week;
 SQL;
 
diff --git a/reports/learners/classes/query_helper.php b/reports/learners/classes/query_helper.php
index 757177c..53f3224 100644
--- a/reports/learners/classes/query_helper.php
+++ b/reports/learners/classes/query_helper.php
@@ -116,7 +116,7 @@ SQL;
                 AND e.courseid = ?
                 AND co.startdate <> 0
                 AND co.visible = 1
-            GROUP BY co.{$groupbychoice}, {$casevalue}
+            GROUP BY co.{$groupbychoice}, {$coursename}, {$casevalue}, co.startdate
             HAVING COUNT(*) > ? AND {$casevalue} <> 0
             ORDER BY users DESC;
 SQL;
@@ -178,7 +178,7 @@ SQL;
             ON e.id = ue.enrolid
         WHERE u.deleted = 0
             AND e.courseid = ?
-        GROUP BY time
+        GROUP BY CASE WHEN ue.timestart < {$timestamp} THEN 0 ELSE 1 END
         ORDER BY time;
 SQL;
 
diff --git a/reports/quiz_assign/classes/query_helper.php b/reports/quiz_assign/classes/query_helper.php
index b921f42..daab96d 100644
--- a/reports/quiz_assign/classes/query_helper.php
+++ b/reports/quiz_assign/classes/query_helper.php
@@ -55,7 +55,7 @@ class query_helper {
         WHERE gi.courseid = ?
             AND gi.itemtype = 'mod'
             AND gi.itemmodule = 'quiz'
-        GROUP BY q.id
+        GROUP BY q.id, q.sumgrades
 SQL;
 
         return $DB->get_records_sql($query, [$courseid]);
diff --git a/reports/weekheatmap/classes/query_helper.php b/reports/weekheatmap/classes/query_helper.php
index e118f5d..a926b1e 100644
--- a/reports/weekheatmap/classes/query_helper.php
+++ b/reports/weekheatmap/classes/query_helper.php
@@ -43,6 +43,8 @@ class query_helper {
             $date = new DateTime();        
             $timezone = $date->getTimezone()->getName();
             $weekstatement = "TO_CHAR(TO_TIMESTAMP(l.timecreated) at time zone '".$timezone."', 'D-HH24')";
+        } elseif ($CFG->dbtype === 'sqlsrv') {
+            $weekstatement = "CONCAT(DATEPART(weekday, DATEADD(SECOND, l.timecreated, '1970-01-01')) - 1, '-', DATEPART(hour, DATEADD(SECOND, l.timecreated + 3600, '1970-01-01')))";
         }
 
         // MySQL returns points where 0-00 => Sun,0-1am; 0-01 => Sun,1-2am; ...
@@ -53,7 +55,7 @@ class query_helper {
             COUNT(1) AS value
         FROM {logstore_lanalytics_log} AS l
             WHERE l.courseid = ?
-        GROUP BY heatpoint
+        GROUP BY {$weekstatement}
         ORDER BY heatpoint
 SQL;
 

[mrkskwsnck]

@thomasdondorf Yes, I′d like to make a PR. I suggest creating a branch for SQL Server marked as experimental.

[mrkskwsnck]

I patched my above Patch #2 (comment) to correct time zone offset. Previously the course week access statistics heatmap showed access statistics one hour early per grid.

[thomasdondorf]

This looks good! Do you want to make a pull request? Otherwise I can also manually apply the patch file.

[mrkskwsnck]

OK, I will create a PR 👍🏼

[mrkskwsnck]

I just created PR #11

[thomasdondorf]

Thanks, I'll look into this in January 👍

I'm closing this. Questions/discussion then in the pull request.