From eb7b9bf89ec3cd209af702d40114d1553d3a3e1c Mon Sep 17 00:00:00 2001
From: Glenn Rice <grice1@missouriwestern.edu>
Date: Mon, 26 Feb 2024 20:58:39 -0600
Subject: [PATCH] Fix a potentially ambiguous column in SQL order by clauses.

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.
---
 lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm | 3 ++-
 lib/WeBWorK/ContentGenerator/ProblemSet.pm            | 8 +++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm b/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm
index 5e6d00a902..e91aeac478 100644
--- a/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm
+++ b/lib/WeBWorK/ContentGenerator/Instructor/UserDetail.pm
@@ -24,6 +24,7 @@ WeBWorK::ContentGenerator::Instructor::UserDetail - Detailed User specific infor
 
 use WeBWorK::Utils qw(x);
 use WeBWorK::Utils::Instructor qw(assignSetToUser);
+use WeBWorK::DB::Utils qw(grok_versionID_from_vsetID_sql);
 use WeBWorK::Debug;
 
 # We use the x function to mark strings for localizaton
@@ -158,7 +159,7 @@ sub initialize ($c) {
 		$c->{mergedVersions}{$setID} = [
 			$db->getMergedSetVersionsWhere(
 				{ user_id => $editForUserID, set_id => { like => "$setID,v\%" } },
-				\q{(SUBSTRING(set_id,INSTR(set_id,',v')+2)+0)}
+				\grok_versionID_from_vsetID_sql($db->{set_version_merged}->sql->_quote('set_id'))
 			)
 		];
 	}
diff --git a/lib/WeBWorK/ContentGenerator/ProblemSet.pm b/lib/WeBWorK/ContentGenerator/ProblemSet.pm
index bb32bad011..6980d4c509 100644
--- a/lib/WeBWorK/ContentGenerator/ProblemSet.pm
+++ b/lib/WeBWorK/ContentGenerator/ProblemSet.pm
@@ -26,6 +26,7 @@ problem set.
 use WeBWorK::Debug;
 use WeBWorK::Utils qw(path_is_subdir is_restricted wwRound before between after grade_set format_set_name_display);
 use WeBWorK::Utils::Rendering qw(renderPG);
+use WeBWorK::DB::Utils qw(grok_versionID_from_vsetID_sql);
 use WeBWorK::Localize;
 
 async sub initialize ($c) {
@@ -195,9 +196,10 @@ sub gateway_body ($c) {
 	my $timeInterval     = $set->time_interval || 0;
 	my @versionData;
 
-	my @setVersions =
-		$db->getMergedSetVersionsWhere({ user_id => $effectiveUser, set_id => { like => $set->set_id . ',v%' } },
-			\q{(SUBSTRING(set_id,INSTR(set_id,',v')+2)+0)});
+	my @setVersions = $db->getMergedSetVersionsWhere(
+		{ user_id => $effectiveUser, set_id => { like => $set->set_id . ',v%' } },
+		\grok_versionID_from_vsetID_sql($db->{set_version_merged}->sql->_quote('set_id'))
+	);
 
 	for my $verSet (@setVersions) {
 		# Count number of versions in current timeInterval