Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.5, 2.1.2, 2.2
    • Fix Version/s: 2.0.6, 2.1.3
    • Component/s: Course
    • Labels:
      None
    • Environment:
      RHEL6, Oracle 11g
    • Database:
      Oracle
    • Testing Instructions:
      Hide

      Using the 4DBs:

      • use the course search facility
      • check no SQL error is thrown and the facility works ok, returning matching courses
      Show
      Using the 4DBs: use the course search facility check no SQL error is thrown and the facility works ok, returning matching courses
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Following the fix for MDL-29496, searching for courses fails under the Oracle database with this error: ORA-00932: inconsistent datatypes: expected CLOB got CHAR

      The problem is the COALESCE(c.summary, ' ') in lib/datalib.php (around line 720) which mixes CLOB and CHAR. A solution is this:

      -    $concat = $DB->sql_concat("COALESCE(c.summary, '". $DB->sql_empty() ."')", "' '", 'c.fullname', "' '", 'c.idnumber', "' '", 'c.shortname');
      +    $concat = $DB->sql_concat("COALESCE(". $DB->sql_compare_text("c.summary", 2000) .", '". $DB->sql_empty() ."')", "' '", 'c.fullname', "' '", 'c.idnumber', "' '", 'c.shortname');

      The 2000 parameter is because by default $DB->sql_compare_text() takes only the first 32 characters of c.summary, which is not very many, but this only affects databases for which $DB->sql_compare_text() isn't a no-op.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Eloy Lafuente (stronk7) added a comment -

            Hi Jonathon,

            good analysis.

            As commented @ MDL-29496 , using $DB->sql_compare_text() is one alternative, but it has the problem of (potentially) trimming contents, so I'm biased to, instead, simply, add something like this:

            if ($DB->get_dbfamiy() == 'oracle') {
                $concat = $DB->sql_concat('c.summary', "' '", 'c.fullname', "' '", 'c.idnumber', "' '", 'c.shortname');
            } else {
                // Current code
            }
            

            and done. As far as Oracle concat results are not ANSI, concatenating NULL does not produce NULL, so it's safe to leave the query as originally was before MDL-29496 arrived, to fix the rest of DBs having ANSI concatenation.

            Sounds ok? IMO it's the simpler way and does not require any extra casting at all. And was supposedly working 2 weeks ago.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Hi Jonathon, good analysis. As commented @ MDL-29496 , using $DB->sql_compare_text() is one alternative, but it has the problem of (potentially) trimming contents, so I'm biased to, instead, simply, add something like this: if ($DB->get_dbfamiy() == 'oracle') { $concat = $DB->sql_concat('c.summary', "' '", 'c.fullname', "' '", 'c.idnumber', "' '", 'c.shortname'); } else { // Current code } and done. As far as Oracle concat results are not ANSI, concatenating NULL does not produce NULL, so it's safe to leave the query as originally was before MDL-29496 arrived, to fix the rest of DBs having ANSI concatenation. Sounds ok? IMO it's the simpler way and does not require any extra casting at all. And was supposedly working 2 weeks ago. Ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Sending this to integration, in the mean time (nor sure if will land this week or next one), would be great to get some feedback.

            Note to integrators: This must be backported to all the branches where MDL-29496 was applied (20 and 21 if I'm not wrong).

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Sending this to integration, in the mean time (nor sure if will land this week or next one), would be great to get some feedback. Note to integrators: This must be backported to all the branches where MDL-29496 was applied (20 and 21 if I'm not wrong). Ciao
            Hide
            Jonathon Fowler added a comment -

            @Eloy: Sounds good to me.

            Show
            Jonathon Fowler added a comment - @Eloy: Sounds good to me.
            Hide
            Sam Hemelryk added a comment -

            Thanks Eloy - this has been integrated now

            Show
            Sam Hemelryk added a comment - Thanks Eloy - this has been integrated now
            Hide
            Rossiani Wijaya added a comment -

            This is working great.

            Test passed.

            Show
            Rossiani Wijaya added a comment - This is working great. Test passed.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Done, your delicious hacks have been sent upstream, many thanks!

            Closing as fixed, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Done, your delicious hacks have been sent upstream, many thanks! Closing as fixed, ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: