Moodle
  1. Moodle
  2. MDL-38091

Error on overwriting My private files with Oracle & SQL*Server

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.4.1
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Files API
    • Labels:
    • Database:
      Oracle
    • Testing Instructions:
      Hide

      Needs testing using oracle, mssql and sqlsrv drivers

      1. Upload a file to your private files area
      2. Save changes
      3. Upload a different file (different content) with the same name
      4. Overwrite the original file
      5. Save changes
      6. VERIFY you see no errors
      7. Repeat this with a file resource in a course that has a reference to the file in private files area that is being updated
      Show
      Needs testing using oracle, mssql and sqlsrv drivers Upload a file to your private files area Save changes Upload a different file (different content) with the same name Overwrite the original file Save changes VERIFY you see no errors Repeat this with a file resource in a course that has a reference to the file in private files area that is being updated
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      wip-MDL-38091-MOODLE_24_STABLE
    • Pull Master Branch:
      wip-MDL-38091-master
    • Rank:
      47891

      Description

      When you try to overwrite your private file with a new file the system display this error:

      ORA-00932: inconsistent datatypes: expected - got CLOB
      
      
      SELECT repositoryid, id FROM c_files_reference WHERE referencehash = :o_param1 and reference = :o_param2 [array ( 'o_param1' ⇒ '8bccfd63767d981f2cc7d76caa0348dd6b219f0e', 'o_param2' ⇒ 'YTo2OntzOjk6ImNvbnRleHRpZCI7aTo1O3M6OToiY29tcG9uZW50IjtzOjQ6InVzZXIiO3M6NjoiaXRlbWlkIjtpOjA7czo4OiJmaWxlYXJlYSI7czo3OiJwcml2YXRlIjtzOjg6ImZpbGVwYXRoIjtzOjE2OiIvUHJvdmEgQ2FydGVsbGEvIjtzOjg6ImZpbGVuYW1lIjtzOjE5OiJhcmdvbWVudG9fdmlvbGEucG5nIjt9', )]
      
      Error code: dmlreadexception
      
      Stack trace:
      
        line 426 of /lib/dml/moodle_database.php: dml_read_exception thrown
        line 274 of /lib/dml/oci_native_moodle_database.php: call to moodle_database->query_end()
        line 1066 of /lib/dml/oci_native_moodle_database.php: call to oci_native_moodle_database->query_end()
        line 1856 of /lib/filestorage/file_storage.php: call to oci_native_moodle_database->get_recordset_sql()
        line 871 of /lib/filelib.php: call to file_storage->update_references_to_storedfile()
        line 296 of /lib/filelib.php: call to file_save_draft_area_files()
        line 70 of /user/files.php: call to file_postupdate_standard_filemanager()
      

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Hi, Sara.

          That does look nasty, however I wasn't able to reproduce it.

          Could you provide some replication steps to follow? Please be specific about where you are getting the file from and if you are using linking.

          Show
          Michael de Raadt added a comment - Hi, Sara. That does look nasty, however I wasn't able to reproduce it. Could you provide some replication steps to follow? Please be specific about where you are getting the file from and if you are using linking.
          Hide
          Sara Cenni added a comment - - edited

          The problem happens only when using ORACLE.

          The steps are the following:
          1) I upload a file in My private files
          2) I upload a different file with the same name
          3) I confirm the overwriting

          Then I get the error.

          The problem seems to be caused by the following query:

          $sql = "SELECT repositoryid, id FROM

          {files_reference} WHERE referencehash = ? and reference = ?";

          line 1854 in lib/filestorage/file_storage.php

          I solve updating the code with this lines:


          + $varcharcontent = $DB->sql_compare_text('reference',4000);

          $sql = "SELECT repositoryid, id FROM {files_reference}

          -WHERE referencehash = ? and reference = ?";

          +WHERE referencehash = ? and $varcharcontent = ?";

          Show
          Sara Cenni added a comment - - edited The problem happens only when using ORACLE. The steps are the following: 1) I upload a file in My private files 2) I upload a different file with the same name 3) I confirm the overwriting Then I get the error. The problem seems to be caused by the following query: $sql = "SELECT repositoryid, id FROM {files_reference} WHERE referencehash = ? and reference = ?"; line 1854 in lib/filestorage/file_storage.php I solve updating the code with this lines: + $varcharcontent = $DB->sql_compare_text('reference',4000); $sql = "SELECT repositoryid, id FROM {files_reference} -WHERE referencehash = ? and reference = ?"; +WHERE referencehash = ? and $varcharcontent = ?";
          Hide
          Michael de Raadt added a comment -

          Hi, Sara.

          I was testing this with Oracle. I am now able to replicate the problem. Trick was to overwrite the file with a file with different content (that would hash differently) but the same name.

          Your fix seems reasonable as the reference field is text. Thanks.

          Show
          Michael de Raadt added a comment - Hi, Sara. I was testing this with Oracle. I am now able to replicate the problem. Trick was to overwrite the file with a file with different content (that would hash differently) but the same name. Your fix seems reasonable as the reference field is text. Thanks.
          Hide
          Michael de Raadt added a comment -

          Actually, the second part of the test is not needed anyway. The referencehash value represents the reference, so there is no need to compare the reference as well. This second part of WHERE test can be removed; it is redundant and probably inefficient (as well as incompatible with different DBs). I tried removing the second part of the test and that worked across DBs.

          I'm assigning this to Marina who wrote the query. Could you check my assumption, Marina?

          Show
          Michael de Raadt added a comment - Actually, the second part of the test is not needed anyway. The referencehash value represents the reference, so there is no need to compare the reference as well. This second part of WHERE test can be removed; it is redundant and probably inefficient (as well as incompatible with different DBs). I tried removing the second part of the test and that worked across DBs. I'm assigning this to Marina who wrote the query. Could you check my assumption, Marina?
          Hide
          Damyon Wiese added a comment -

          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.

          Thanks!

          Show
          Damyon Wiese added a comment - 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. Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note exactly the same happens against SQL*Server databases, so I'm amended the title and the testing instructions to cover both the mssql and sqlsrv drivers.

          The patch looks perfect, that reference seems 100% redundant.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Note exactly the same happens against SQL*Server databases, so I'm amended the title and the testing instructions to cover both the mssql and sqlsrv drivers. The patch looks perfect, that reference seems 100% redundant. Ciao
          Hide
          Dan Poltawski added a comment -

          Integrated to master, 24 and 23 - thanks Marina

          Show
          Dan Poltawski added a comment - Integrated to master, 24 and 23 - thanks Marina
          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Marina,

          Works great.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Marina, Works great.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Because

          A
          MARVELOUS
          A       U
          Z  YOU  P
          I  ARE  E
          N  PPL  R
          G       B
            TNKS! 
          

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Because A MARVELOUS A U Z YOU P I ARE E N PPL R G B TNKS! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: