Moodle

SQL error with ambiguous column definition

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major 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??

Attachments

  1. Text File
    patch_ambiguous_col_2.0.patch
    06/Apr/10 7:32 PM
    2 kB
    Tim Hunt
  2. Text File
    patch_ambiguous_col.patch
    06/Apr/10 7:13 PM
    2 kB
    Tim Hunt
  3. Text File
    patch_ambiguous_col.patch
    16/Mar/10 8:56 PM
    2 kB
    David Binney
  4. File
    patch_lib_questionlib.php
    16/Mar/10 10:01 AM
    1 kB
    David Binney

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
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
Permalink
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
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
Permalink
David Binney added a comment -

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

Show
David Binney added a comment - This is the updated patch file with the changes specified in the tracker
Hide
Permalink
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
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
Permalink
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
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
Permalink
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
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
Permalink
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
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
Permalink
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
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
Permalink
Tim Hunt added a comment -

Finally committed. Thanks David.

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

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.