search

openwebwork / webwork2

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

Fix a potentially ambiguous column in SQL order by clauses. #2343

Closed drgrice1 closed 5 months ago

drgrice1 commented 5 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.

drgrice1 commented 5 months ago

Note that I have tested that this works with the MariaDB database and both the DBD::MariaDB and DBD::mysql interface drivers. Although the current code also works with both of those configurations. What is needed is for someone to test this with mysql, and see if they can reproduce the issue with the develop branch (or main), and if this pull request fixes the issue.

drgrice1 commented 5 months ago

Update on testing this.

I built a docker image that uses mysql 8.0 with the DBD:mysql interface driver and the current develop branch. With that build I was able to reproduce the issue. I created a test, and upon opening the overview page for that test I get the error DBD::mysql::st execute failed: Column 'set_id' in order clause is ambiguous at /opt/webwork/webwork2/lib/WeBWorK/DB/Schema/NewSQL/Std.pm line 930.

I then rebuilt the docker image switching to the branch for this pull request, and the error did not occur.

So this is the correct fix. I recommend the hotfix be considered.

It is becoming increasingly difficult to support using mysql, particularly since all of the active webwork2 developers have now switched to using MariaDB.