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

SQL error with ambiguous column definition

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.7
    • Fix Version/s: 1.9.9
    • Component/s: Questions
    • Labels:
      None
    • Database:
      Any
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE

      Description

      There is an invalid query in the ./lib/question.php file with a reference to an ambiguous column definition which appears twice in the select. Oracle will not accept this as a valid query and errors with :

      SQL ORA-00918: column ambiguously defined in /usr/local/moodle-src/rel-20100113-4/lib/dmllib.php on line 481. STATEMENT: SELECT n.questionid as question, s.*, n.sumpenalty FROM m_question_states s, m_question_sessions n WHERE s.id = n.newgraded AND n.attemptid = '67706' AND n.questionid = '31777' with limits (-1, 1), referer: http://moodle.cqu.edu.au/mod/quiz/view.php?id=47336

      I have fixed this in the two places within that file by removing the reference to the duplicated column and have included a patch.

      Also, i have not submitted a patch before so i would be willing to have any comments about format of patch....etc??

        Gliffy Diagrams

        1. patch_ambiguous_col_2.0.patch
          2 kB
          Tim Hunt
        2. patch_ambiguous_col.patch
          2 kB
          Tim Hunt
        3. patch_ambiguous_col.patch
          2 kB
          David Binney
        4. patch_lib_questionlib.php
          1 kB
          David Binney

          Activity

          Hide
          timhunt Tim Hunt added a comment -

          That patch won't work. In SQL queries in Moodle, the first column is significant. It gets used to index the array that is returned.

          I think the correct solution it to explicitly write out all the s.... columns (except for question) to avoid the duplication.

          The patch looks find. The only comment I would make is that it is best to give it a file extension different from .php. I often use .patch.txt.

          Show
          timhunt Tim Hunt added a comment - That patch won't work. In SQL queries in Moodle, the first column is significant. It gets used to index the array that is returned. I think the correct solution it to explicitly write out all the s.... columns (except for question) to avoid the duplication. The patch looks find. The only comment I would make is that it is best to give it a file extension different from .php. I often use .patch.txt.
          Hide
          donkeyx David Binney added a comment -

          Ok cool, I was not sure of the implication or why it was done this way. I will update the code again and re-create the patch for you.

          Show
          donkeyx David Binney added a comment - Ok cool, I was not sure of the implication or why it was done this way. I will update the code again and re-create the patch for you.
          Hide
          donkeyx David Binney added a comment -

          This is the updated patch file with the changes specified in the tracker

          Show
          donkeyx David Binney added a comment - This is the updated patch file with the changes specified in the tracker
          Hide
          donkeyx David Binney added a comment -

          Hey Tim, I made the specified changes but was just unsure of the replacement of the questionid for the quesiton but i am sure you will let me know if that is no good

          Show
          donkeyx David Binney added a comment - Hey Tim, I made the specified changes but was just unsure of the replacement of the questionid for the quesiton but i am sure you will let me know if that is no good
          Hide
          timhunt Tim Hunt added a comment -

          Well, tabs for indent a bad (according to Development:Coding_style#Indentation). But apart from that it looks fine. I will try to commit it soon, but today is testing day before the weekly build.

          Show
          timhunt Tim Hunt added a comment - Well, tabs for indent a bad (according to Development:Coding_style#Indentation). But apart from that it looks fine. I will try to commit it soon, but today is testing day before the weekly build.
          Hide
          donkeyx David Binney added a comment -

          Good eyes tim. Since switching to the mac I have not even looked for switch spaces to tabs on textmate (soft tabs).. damn

          Show
          donkeyx David Binney added a comment - Good eyes tim. Since switching to the mac I have not even looked for switch spaces to tabs on textmate (soft tabs).. damn
          Hide
          timhunt Tim Hunt added a comment -

          Oops, I forgot all about this.

          Attached is a cleaned up patch. I was about to commit this to CVS, when I remembered that it is testing day again.

          Hopefully I will remember tomorrow.

          Show
          timhunt Tim Hunt added a comment - Oops, I forgot all about this. Attached is a cleaned up patch. I was about to commit this to CVS, when I remembered that it is testing day again. Hopefully I will remember tomorrow.
          Hide
          timhunt Tim Hunt added a comment -

          And I think this is the correct merge to Moodle 2.0, where the code has changed a bit.

          Show
          timhunt Tim Hunt added a comment - And I think this is the correct merge to Moodle 2.0, where the code has changed a bit.
          Hide
          timhunt Tim Hunt added a comment -

          Finally committed. Thanks David.

          Show
          timhunt Tim Hunt added a comment - Finally committed. Thanks David.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                8/Jun/10