Moodle
  1. Moodle
  2. MDL-28443 Action logging improvements META
  3. MDL-39068

Config log sort by 'value' and 'old value' doesn't work in MSSQL

    Details

    • Testing Instructions:
      Hide

      Against ALL databases (this was known to be failing under mssql/sqlsrv & oracle drivers:

      1) Go to the admin -> report -> config changes report.
      2) Verify that ordering by "date", "first name" and "surname" continue working.
      3) TEST: Verify that ordering by "new value" and "original value" are working ok on. Up and down (ASC and DESC).

      That's all. Ciao

      Show
      Against ALL databases (this was known to be failing under mssql/sqlsrv & oracle drivers: 1) Go to the admin -> report -> config changes report. 2) Verify that ordering by "date", "first name" and "surname" continue working. 3) TEST: Verify that ordering by "new value" and "original value" are working ok on. Up and down (ASC and DESC). That's all. Ciao
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
    • Pull Master Branch:
    • Rank:
      49192

      Description

      http://localhost/report/configlog/index.php?sort=value&dir=DESC&page=0&perpage=30

      Returns database error:
      Debug info: The text, ntext, and image data types cannot be compared or sorted, except when using IS NULL or LIKE operator.
      SELECT TOP 30 u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email,
      cl.timemodified, cl.plugin, cl.name, cl.value, cl.oldvalue
      FROM mdl_config_log cl
      JOIN mdl_user u ON u.id = cl.userid
      ORDER BY cl.value DESC
      [array (
      )]
      Error code: dmlreadexception
      Stack trace:

      line 426 of \lib\dml\moodle_database.php: dml_read_exception thrown
      line 256 of \lib\dml\mssql_native_moodle_database.php: call to moodle_database->query_end()
      line 713 of \lib\dml\mssql_native_moodle_database.php: call to mssql_native_moodle_database->query_end()
      line 113 of \report\configlog\index.php: call to mssql_native_moodle_database->get_recordset_sql()

        Activity

        Hide
        Valerii Kuznetsov added a comment - - edited

        Locally it can be fixed by altering 'config_log.value' and 'config_log.oldvalue' to type 'nvarchar(max)'
        or
        by patch

        "report/configlog/index.php"
        diff --git a/report/configlog/index.php b/report/configlog/index.php
        index 8372727..22fa747 100644
        --- a/report/configlog/index.php
        +++ b/report/configlog/index.php
        @@ -100,7 +100,12 @@ $table->data  = array();
         if ($sort == 'firstname' or $sort == 'lastname') {
             $orderby = "u.$sort $dir";
         } else {
        -    $orderby = "cl.$sort $dir";
        +    if (strpos($sort, 'value') !== false) {
        +        // MSSQL ntext field issue
        +        $orderby = $DB->sql_order_by_text("cl.$sort", 255).' '.$dir;
        +    } else {
        +        $orderby = "cl.$sort $dir";
        +    }
         }
        
        Show
        Valerii Kuznetsov added a comment - - edited Locally it can be fixed by altering 'config_log.value' and 'config_log.oldvalue' to type 'nvarchar(max)' or by patch "report/configlog/index.php" diff --git a/report/configlog/index.php b/report/configlog/index.php index 8372727..22fa747 100644 --- a/report/configlog/index.php +++ b/report/configlog/index.php @@ -100,7 +100,12 @@ $table->data = array(); if ($sort == 'firstname' or $sort == 'lastname') { $orderby = "u.$sort $dir" ; } else { - $orderby = "cl.$sort $dir" ; + if (strpos($sort, 'value') !== false ) { + // MSSQL ntext field issue + $orderby = $DB->sql_order_by_text( "cl.$sort" , 255).' '.$dir; + } else { + $orderby = "cl.$sort $dir" ; + } }
        Hide
        Rajesh Taneja added a comment -

        Thanks for reporting this and providing patch,

        I have assigned this to Petr, as he is the best person to comment.

        Show
        Rajesh Taneja added a comment - Thanks for reporting this and providing patch, I have assigned this to Petr, as he is the best person to comment.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for the report and the fix, Valerii, I've ended changing it a bit, but yours was perfectly valid.

        Sending to integration, typical ORDER BY text problem under mssql/sqlsrv and oracle. Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for the report and the fix, Valerii, I've ended changing it a bit, but yours was perfectly valid. Sending to integration, typical ORDER BY text problem under mssql/sqlsrv and oracle. Ciao
        Hide
        Damyon Wiese added a comment -

        Thanks Valerii and Eloy,

        This has been integrated to master, 24 and 23 branches. I tested postgres on 23, 24 and master and MSSQL on master branches when integrating.

        Show
        Damyon Wiese added a comment - Thanks Valerii and Eloy, This has been integrated to master, 24 and 23 branches. I tested postgres on 23, 24 and master and MSSQL on master branches when integrating.
        Hide
        Michael de Raadt added a comment -

        Test result: Success!

        Tested in 2.3, 2.4 and master using MySQL, PostgreSQL, SQL Server and Oracle.

        All column sort without errors.

        Show
        Michael de Raadt added a comment - Test result: Success! Tested in 2.3, 2.4 and master using MySQL, PostgreSQL, SQL Server and Oracle. All column sort without errors.
        Hide
        Dan Poltawski added a comment -

        Blooming Marvelous! It's time for a knees up - your changes are upstream!

        Thanks for making Moodle better!

        Toodle pip

        Show
        Dan Poltawski added a comment - Blooming Marvelous! It's time for a knees up - your changes are upstream! Thanks for making Moodle better! Toodle pip

          People

          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: