search

openwebwork / webwork2

Course management front end for WeBWorK
http://webwork.maa.org/wiki/Main_Page
Other
145 stars 165 forks source link

Fix a potentially ambiguous column in SQL order by clauses (hotfix). #2344

Closed drgrice1 closed 7 months ago

drgrice1 commented 7 months ago

In WeBWorK::ContentGenerator::Instructor::UserDetail and WeBWorK::ContentGenerator::Instructor::ProblemSet I used q{(SUBSTRING(set_id,INSTR(set_id,',v')+2)+0)} for the order by clause in getMergedSetVersionsWhere calls. The problem is that getMergedSetVersionsWhere performs an inner join on the row from the user_set table that is the user's versioned set, the row from that same table that is the user's template set, and the global template set from the set tabel. Apparently some database configurations are lenient and assume the first table, but others do not.

So this replaces that with grok_versionID_from_vsetID_sql($db->{set_version_merged}->sql->_quote('set_id')). The _quote call ensures that the set_id column is selected from the primary table (the row that is the user's versioned set) since it is passed through the transform_all method of WeBWorK::DB::Schema::NewSQL::Merge.

This will most likely fix #2341. Although I am can't (easily) test this, and can't reproduce the issue reported there. I suspect that this occurs when mysql is used instead of MariaDB. I suspect that MariaDB is more lenient on this.

This is for consideration for a hotfix if this does fix the issue.

Alex-Jordan commented 7 months ago

If the people at uwosh try this and it works, and by now you have successfully tested it with mysql (in #2343) then I'm inclined to just merge it. It is highly unlikely I will find time to set everything up to test with mysql anytime soon. Eventually I might, but it feels like a task I would be slow to complete once I get started.

So this is my unofficial "approval". Maybe the other team members can do a proper test though.

drgrice1 commented 7 months ago

@Alex-Jordan: Could you make it a semi-official approval with an actual Github approval? I have set up Github so that two approvals are required for a merge to the main branch (i.e., a hotfix). An important test that you can do is that this pull request still works with MariaDB. That is at least as important as this working with mysql.

Alex-Jordan commented 7 months ago

OK, I'll try that sometime not too much later, but could be as late as Thursday. I have to pause WW development and catch up with some local stuff.