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

          Attachments

            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