Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29815

When I choose grading from navigation bar, I got "Debug info: ORA-00904: "SUMMARYSTATE": invalid identifier"

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2
    • Fix Version/s: 2.1.3
    • Component/s: Quiz
    • Labels:
    • Environment:
      Oracle database
    • Database:
      Oracle
    • Testing Instructions:
      Hide

      1.Use Oracle as backend database.
      2.Create a quiz use some manually grading questions.
      3.From navigation menu, choose "Grading" in this quiz.
      4.Run database functional tests against the 5 drivers. Ignoring others, test that there is not errors related to test_get_records_sql_complicated tests.

      Also quickly check that it is not broken on at least one other DB.

      Show
      1.Use Oracle as backend database. 2.Create a quiz use some manually grading questions. 3.From navigation menu, choose "Grading" in this quiz. 4.Run database functional tests against the 5 drivers. Ignoring others, test that there is not errors related to test_get_records_sql_complicated tests. Also quickly check that it is not broken on at least one other DB.
    • Workaround:
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      I created a quiz, all questions are graded manually. When I click the "Grading" menu from navigation bar, I get errors like this:

      Debug info: ORA-00904: "SUMMARYSTATE": invalid identifier
       
      SELECT
      qa.slot,
      qa.questionid,
      q.name,
      CASE qas.state
      WHEN 'notstarted' THEN 'inprogress'
      WHEN 'unprocessed' THEN 'inprogress'
      WHEN 'todo' THEN 'inprogress'
      WHEN 'invalid' THEN 'inprogress'
      WHEN 'complete' THEN 'inprogress'
      WHEN 'needsgrading' THEN 'needsgrading'
      WHEN 'finished' THEN 'autograded'
      WHEN 'gaveup' THEN 'autograded'
      WHEN 'gradedwrong' THEN 'autograded'
      WHEN 'gradedpartial' THEN 'autograded'
      WHEN 'gradedright' THEN 'autograded'
      WHEN 'manfinished' THEN 'manuallygraded'
      WHEN 'mangaveup' THEN 'manuallygraded'
      WHEN 'mangrwrong' THEN 'manuallygraded'
      WHEN 'mangrpartial' THEN 'manuallygraded'
      WHEN 'mangrright' THEN 'manuallygraded'
       
      END AS summarystate,
      COUNT(1) AS numattempts
       
      FROM m_quiz_attempts quiza
      JOIN m_question_attempts qa ON qa.questionusageid = quiza.uniqueid
      JOIN m_question_attempt_steps qas ON
      qas.id = (
      SELECT MAX(id)
      FROM m_question_attempt_steps
      WHERE questionattemptid = qa.id
      )
      JOIN m_question q ON q.id = qa.questionid
       
      WHERE
      quiza.quiz = :o_mangrquizid AND
      quiza.preview = 0 AND
      quiza.timefinish <> 0 AND
      qa.slot IN (:o_slot15,:o_slot16,:o_slot17,:o_slot18,:o_slot19)
       
      GROUP BY
      qa.slot,
      qa.questionid,
      q.name,
      q.id,
      summarystate
       
      ORDER BY
      qa.slot,
      qa.questionid,
      q.name,
      q.id
       
      [array (
      'o_mangrquizid' => '148',
      'o_slot15' => 2,
      'o_slot16' => 3,
      'o_slot17' => 4,
      'o_slot18' => 5,
      'o_slot19' => 6,
      )]

      The problem is Oracle don't support alias in group by statement, and it'll report "SUMMARYSTATE" as invalid identifier, because "SUMMARYSTATE" is alias. I changed some code in question/engine/datalib.php:

      question/engine/datalib.php line347-377

      SELECT
          qa.slot,
          qa.questionid,
          q.name,
          CASE qas.state
              {$this->full_states_to_summary_state_sql()}
          END AS summarystate,
          COUNT(1) AS numattempts
       
      FROM {$qubaids->from_question_attempts('qa')}
      JOIN {question_attempt_steps} qas ON
              qas.id = {$this->latest_step_for_qa_subquery()}
      JOIN {question} q ON q.id = qa.questionid
       
      WHERE
          {$qubaids->where()} AND
          qa.slot $slottest
       
      GROUP BY
          qa.slot,
          qa.questionid,
          q.name,
          q.id,
          CASE qas.state                                                                                                                            
          WHEN 'notstarted' THEN 'inprogress'                                                                                                       
          WHEN 'unprocessed' THEN 'inprogress'                                                                                                      
          WHEN 'todo' THEN 'inprogress'                                                                                                             
          WHEN 'invalid' THEN 'inprogress'                                                                                                          
          WHEN 'complete' THEN 'inprogress'                                                                                                         
          WHEN 'needsgrading' THEN 'needsgrading'                                                                                                   
          WHEN 'finished' THEN 'autograded'                                                                                                         
          WHEN 'gaveup' THEN 'autograded'                                                                                                           
          WHEN 'gradedwrong' THEN 'autograded'                                                                                                      
          WHEN 'gradedpartial' THEN 'autograded'                                                                                                    
          WHEN 'gradedright' THEN 'autograded'                                                                                                      
          WHEN 'manfinished' THEN 'manuallygraded'                                                                                                  
          WHEN 'mangaveup' THEN 'manuallygraded'                                                                                                    
          WHEN 'mangrwrong' THEN 'manuallygraded'                                                                                                   
          WHEN 'mangrpartial' THEN 'manuallygraded'                                                                                                 
          WHEN 'mangrright' THEN 'manuallygraded'                                                                                                   
          END      
       
      ORDER BY
          qa.slot,
          qa.questionid,
          q.name,
          q.id
              ", $params + $qubaids->from_where_params());

      Things go wright now. But I still waiting Tim provide an elegant solution.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              timhunt Tim Hunt added a comment -

              Grrr!

              The only elegant solution I can think of is:

              1. install Postgres
              2. ditch Oracle
              3. save $$$$ in excessive license fees,
                but that is probably not the kind of solution you were looking for.

              I will use something like your inelegant fix to fix the code.

              Show
              timhunt Tim Hunt added a comment - Grrr! The only elegant solution I can think of is: install Postgres ditch Oracle save $$$$ in excessive license fees, but that is probably not the kind of solution you were looking for. I will use something like your inelegant fix to fix the code.
              Hide
              luyanfei Yanfei Lu added a comment -

              Ha!
              This one will be a little better:

              GROUP BY
                  qa.slot,
                  qa.questionid,
                  q.name,
                  q.id,
                  CASE qas.state
                      {$this->full_states_to_summary_state_sql()}
                  END  

              Show
              luyanfei Yanfei Lu added a comment - Ha! This one will be a little better: GROUP BY qa.slot, qa.questionid, q.name, q.id, CASE qas.state {$this->full_states_to_summary_state_sql()} END
              Hide
              timhunt Tim Hunt added a comment -

              Thanks for working out the elegant fix Yanfei.

              Show
              timhunt Tim Hunt added a comment - Thanks for working out the elegant fix Yanfei.
              Hide
              stronk7 Eloy Lafuente (stronk7) 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.

              TIA and ciao

              Show
              stronk7 Eloy Lafuente (stronk7) 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. TIA and ciao
              Hide
              nebgor Aparup Banerjee added a comment -

              setting the Oracle master as integrator.

              Show
              nebgor Aparup Banerjee added a comment - setting the Oracle master as integrator.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited

              1) Just guessing if simply grouping by qas.state will lead to same results (if the conversion is always 1:1). Forget, just saw in the complete CASES above that 1:1 in not happening here, so simpler grouping won't work.

              2) Apart from that, I've been looking if that method is covered by any tests and I'm afraid it is not, so we'll need to proceed with testing instructions against all DBs or, alternatively, suggest to add and run one extra test to test_get_records_sql_complicated() to check that CASE statements with multiple WHEN and string values are cross-db. I guess they are, but... better if we have that tested. Note there are also one similar group by test there (but simpler and with integers).

              I'm halting this. If nobody has created the extra test tomorrow, I'll try to do do.

              Ciao

              Edited: Forget about 1) above, cannot apply here.

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - - edited 1) Just guessing if simply grouping by qas.state will lead to same results (if the conversion is always 1:1). Forget, just saw in the complete CASES above that 1:1 in not happening here, so simpler grouping won't work. 2) Apart from that, I've been looking if that method is covered by any tests and I'm afraid it is not, so we'll need to proceed with testing instructions against all DBs or, alternatively, suggest to add and run one extra test to test_get_records_sql_complicated() to check that CASE statements with multiple WHEN and string values are cross-db. I guess they are, but... better if we have that tested. Note there are also one similar group by test there (but simpler and with integers). I'm halting this. If nobody has created the extra test tomorrow, I'll try to do do. Ciao Edited: Forget about 1) above, cannot apply here.
              Hide
              timhunt Tim Hunt added a comment -

              Eloy, a new unit test of the DB lib is a good idea, but surely a separate issue.

              CASE is just an expression in SQL, and you can GROUP BY any expression. I would be amazed in this broke.

              Yanfei Lu has tested on Oracle, and I have tested on Postgres, so that only leaves MySQL and MS SQL.

              I really think this fix should be integrated.

              Show
              timhunt Tim Hunt added a comment - Eloy, a new unit test of the DB lib is a good idea, but surely a separate issue. CASE is just an expression in SQL, and you can GROUP BY any expression. I would be amazed in this broke. Yanfei Lu has tested on Oracle, and I have tested on Postgres, so that only leaves MySQL and MS SQL. I really think this fix should be integrated.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              I've added and tested (works!) this agains the mysql, pgsql, mssql and oci:

              http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=44a96306238bddc38864b37f4df4bb64312b8aa2

              Integrating...

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - I've added and tested (works!) this agains the mysql, pgsql, mssql and oci: http://git.moodle.org/gw?p=integration.git;a=commitdiff;h=44a96306238bddc38864b37f4df4bb64312b8aa2 Integrating...
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Integrated, thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
              Hide
              timhunt Tim Hunt added a comment -

              Yay! thanks Eloy.

              Show
              timhunt Tim Hunt added a comment - Yay! thanks Eloy.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks guys - test passed.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks guys - test passed.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

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

              Closing as fixed, ciao

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

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    28/Nov/11