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:
    • Rank:
      19447

      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.

        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: