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

          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