Fix a potentially ambiguous column in SQL order by clauses (hotfix).#2344
Conversation
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 openwebwork#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.
|
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. |
|
@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. |
|
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. |
Alex-Jordan
left a comment
Alex-Jordan
left a comment
There was a problem hiding this comment.
I'm good to merge this.
3 participants
In
WeBWorK::ContentGenerator::Instructor::UserDetailandWeBWorK::ContentGenerator::Instructor::ProblemSetI usedq{(SUBSTRING(set_id,INSTR(set_id,',v')+2)+0)}for the order by clause ingetMergedSetVersionsWherecalls. The problem is thatgetMergedSetVersionsWhereperforms an inner join on the row from theuser_settable 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 thesettabel. 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_quotecall ensures that theset_idcolumn is selected from the primary table (the row that is the user's versioned set) since it is passed through thetransform_allmethod ofWeBWorK::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
mysqlis used instead ofMariaDB. I suspect thatMariaDBis more lenient on this.This is for consideration for a hotfix if this does fix the issue.