diff --git a/bin/upgrade-database-to-utf8mb4.pl b/bin/upgrade-database-to-utf8mb4.pl
index 4765ce28db..3751a2fd30 100755
--- a/bin/upgrade-database-to-utf8mb4.pl
+++ b/bin/upgrade-database-to-utf8mb4.pl
@@ -165,7 +165,7 @@ BEGIN
my $dbuser = shell_quote($ce->{database_username});
my $dbpass = $ce->{database_password};
-$ENV{'MYSQL_PWD'} = $dbpass;
+local $ENV{MYSQL_PWD} = $dbpass;
if (!$no_backup) {
# Backup the database
@@ -212,7 +212,7 @@ BEGIN
},
);
-my $db = new WeBWorK::DB($ce->{dbLayouts}{ $ce->{dbLayoutName} });
+my $db = WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} });
my @table_types = sort(grep { !$db->{$_}{params}{non_native} } keys %$db);
sub checkAndUpdateTableColumnTypes {
@@ -222,29 +222,30 @@ sub checkAndUpdateTableColumnTypes {
print "\tChecking '$table' (pass $pass)\n" if $verbose;
my $schema_field_data = $db->{$table_type}{record}->FIELD_DATA;
- for my $field (keys %$schema_field_data) {
- my $field_name = $db->{$table_type}{params}{fieldOverride}{$field} || $field;
- my @name_type = @{
+ for my $field_name (keys %$schema_field_data) {
+ my @name_type = @{
$dbh->selectall_arrayref(
"SELECT COLUMN_TYPE FROM INFORMATION_SCHEMA.COLUMNS "
. "WHERE TABLE_SCHEMA='$dbname' AND TABLE_NAME='$table' AND COLUMN_NAME='$field_name';"
)
};
- print("\t\tThe '$field_name' column is missing from '$table'.\n"
- . "\t\tYou should upgrade the course via course administration to fix this.\n"
- . "\t\tYou may need to run this script again after doing that.\n"), next
- if !exists($name_type[0][0]);
+ if (!exists($name_type[0][0])) {
+ print("\t\tThe '$field_name' column is missing from '$table'.\n"
+ . "\t\tYou should upgrade the course via course administration to fix this.\n"
+ . "\t\tYou may need to run this script again after doing that.\n");
+ next;
+ }
my $data_type = $name_type[0][0];
next if !$data_type;
$data_type =~ s/\(\d*\)$// if $data_type =~ /^(big|small)?int\(\d*\)$/;
$data_type = lc($data_type);
- my $schema_data_type = lc($schema_field_data->{$field}{type} =~ s/ .*$//r);
+ my $schema_data_type = lc($schema_field_data->{$field_name}{type} =~ s/ .*$//r);
if ($data_type ne $schema_data_type) {
print "\t\tUpdating data type for column '$field_name' in table '$table'\n" if $verbose;
print "\t\t\t$data_type -> $schema_data_type\n" if $verbose;
- eval { $dbh->do("ALTER TABLE `$table` MODIFY $field_name $schema_field_data->{$field}{type};"); };
+ eval { $dbh->do("ALTER TABLE `$table` MODIFY $field_name $schema_field_data->{$field_name}{type};"); };
my $indent = $verbose ? "\t\t" : "";
die("${indent}Failed to modify '$field_name' in '$table' from '$data_type' to '$schema_data_type.\n"
. "${indent}It is recommended that you restore a database backup. Make note of the\n"
@@ -286,8 +287,10 @@ sub checkAndChangeTableCharacterSet {
my $error = 0;
for my $course (@courses) {
- print("The course '$course' does not exist on the server\n"), next
- if !grep($course eq $_, @server_courses);
+ if (!grep { $course eq $_ } @server_courses) {
+ print("The course '$course' does not exist on the server\n");
+ next;
+ }
print "Checking tables for '$course'\n" if $verbose;
for my $table_type (@table_types) {
diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm
index fdb4463636..63e14daf58 100644
--- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm
+++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm
@@ -1339,7 +1339,7 @@ sub upgrade_course_confirm ($c) {
my @upgrade_courseIDs = $c->param('upgrade_courseIDs');
- my ($extra_database_tables_exist, $extra_database_fields_exist) = (0, 0);
+ my ($extra_database_tables_exist, $extra_database_fields_exist, $incorrect_type_database_fields_exist) = (0, 0, 0);
my $status_output = $c->c;
@@ -1354,8 +1354,9 @@ sub upgrade_course_confirm ($c) {
# Report on database status
my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID);
- my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) =
- $c->formatReportOnDatabaseTables($dbStatus, $upgrade_courseID);
+ my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes,
+ $incorrect_type_database_fields, $db_report)
+ = $c->formatReportOnDatabaseTables($dbStatus, $upgrade_courseID);
my $course_output = $c->c;
@@ -1428,6 +1429,24 @@ sub upgrade_course_confirm ($c) {
);
}
+ if ($incorrect_type_database_fields) {
+ $incorrect_type_database_fields_exist = 1;
+ push(
+ @$course_output,
+ $c->tag(
+ 'p',
+ class => 'text-danger fw-bold',
+ $c->maketext(
+ 'There are database fields that do not have the same type as the field defined in the schema '
+ . 'for at least one table. Check the checkbox by the field to change its type when '
+ . 'upgrading the course. Warning: This can fail which may corrupt the table. If you have '
+ . 'not archived this course, then do that now before upgrading if you want to change the '
+ . 'type of any of these fields.'
+ )
+ )
+ );
+ }
+
# Report on directory status
my ($directories_ok, $directory_report) = checkCourseDirectories($ce2);
push(@$course_output, $c->tag('div', class => 'mb-2', $c->maketext('Directory structure:')));
@@ -1464,10 +1483,11 @@ sub upgrade_course_confirm ($c) {
return $c->include(
'ContentGenerator/CourseAdmin/upgrade_course_confirm',
- upgrade_courseIDs => \@upgrade_courseIDs,
- extra_database_tables_exist => $extra_database_tables_exist,
- extra_database_fields_exist => $extra_database_fields_exist,
- status_output => $status_output->join('')
+ upgrade_courseIDs => \@upgrade_courseIDs,
+ extra_database_tables_exist => $extra_database_tables_exist,
+ extra_database_fields_exist => $extra_database_fields_exist,
+ incorrect_type_database_fields_exist => $incorrect_type_database_fields_exist,
+ status_output => $status_output->join('')
);
}
@@ -1503,8 +1523,10 @@ sub do_upgrade_course ($c) {
push(
@upgrade_report,
$CIchecker->updateTableFields(
- $upgrade_courseID, $table_name,
- [ ($c->param("$upgrade_courseID.$table_name.delete_fieldIDs")) ]
+ $upgrade_courseID,
+ $table_name,
+ [ ($c->param("$upgrade_courseID.$table_name.delete_fieldIDs")) ],
+ [ ($c->param("$upgrade_courseID.$table_name.fix_type_fieldIDs")) ],
)
);
}
@@ -1512,8 +1534,9 @@ sub do_upgrade_course ($c) {
# Analyze database status and prepare status report
($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID);
- my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) =
- $c->formatReportOnDatabaseTables($dbStatus);
+ my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes,
+ $incorrect_type_database_fields, $db_report)
+ = $c->formatReportOnDatabaseTables($dbStatus);
# Prepend course name
$db_report = $c->c($c->tag('div', class => 'mb-2', $c->maketext('Database:')), $db_report);
@@ -1541,6 +1564,20 @@ sub do_upgrade_course ($c) {
);
}
+ if ($incorrect_type_database_fields) {
+ push(
+ @$db_report,
+ $c->tag(
+ 'p',
+ class => 'text-danger fw-bold',
+ $c->maketext(
+ 'There are database fields that do not have the same type as the '
+ . 'field defined in the schema for at least one table.'
+ )
+ )
+ );
+ }
+
# Add missing directories and prepare report on directory status
my $dir_update_messages = updateCourseDirectories($ce2); # Needs more error messages
my ($directories_ok, $directory_report) = checkCourseDirectories($ce2);
@@ -2684,10 +2721,11 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
)
);
- my $all_tables_ok = 1;
- my $extra_database_tables = 0;
- my $extra_database_fields = 0;
- my $rebuild_table_indexes = 0;
+ my $all_tables_ok = 1;
+ my $extra_database_tables = 0;
+ my $extra_database_fields = 0;
+ my $rebuild_table_indexes = 0;
+ my $incorrect_type_database_fields = 0;
my $db_report = $c->c;
@@ -2728,9 +2766,35 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
} else {
$extra_database_fields = 1;
}
- if ($fieldInfo{$key}[1]) {
- push(@$field_report, $c->hidden_field("$courseID.$table.delete_fieldIDs" => $key));
- } else {
+ if (defined $courseID) {
+ if ($fieldInfo{$key}[1]) {
+ push(@$field_report, $c->hidden_field("$courseID.$table.delete_fieldIDs" => $key));
+ } else {
+ push(
+ @$field_report,
+ $c->tag(
+ 'span',
+ class => 'form-check d-inline-block',
+ $c->tag(
+ 'label',
+ class => 'form-check-label',
+ $c->c(
+ $c->check_box(
+ "$courseID.$table.delete_fieldIDs" => $key,
+ class => 'form-check-input'
+ ),
+ $c->maketext('Delete field when upgrading')
+ )->join('')
+ )
+ )
+ );
+ }
+ }
+ } elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A) {
+ $all_tables_ok = 0;
+ } elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B) {
+ $incorrect_type_database_fields = 1;
+ if (defined $courseID) {
push(
@$field_report,
$c->tag(
@@ -2741,17 +2805,15 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
class => 'form-check-label',
$c->c(
$c->check_box(
- "$courseID.$table.delete_fieldIDs" => $key,
- class => 'form-check-input'
+ "$courseID.$table.fix_type_fieldIDs" => $key,
+ class => 'form-check-input'
),
- $c->maketext('Delete field when upgrading')
+ $c->maketext('Change type of field when upgrading')
)->join('')
)
)
);
}
- } elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A) {
- $all_tables_ok = 0;
}
push(@$fields_report, $c->tag('li', $field_report->join('')));
}
@@ -2765,8 +2827,9 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) {
push(@$db_report, $c->tag('p', class => 'text-success', $c->maketext('Database tables are ok'))) if $all_tables_ok;
return (
- $all_tables_ok, $extra_database_tables, $extra_database_fields,
- $rebuild_table_indexes, $db_report->join('')
+ $all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes,
+ $incorrect_type_database_fields,
+ $db_report->join('')
);
}
diff --git a/lib/WeBWorK/DB/Schema/NewSQL/Std.pm b/lib/WeBWorK/DB/Schema/NewSQL/Std.pm
index 497db424df..794713200c 100644
--- a/lib/WeBWorK/DB/Schema/NewSQL/Std.pm
+++ b/lib/WeBWorK/DB/Schema/NewSQL/Std.pm
@@ -368,6 +368,24 @@ sub _drop_column_field_stmt {
return "Alter table `$sql_table_name` drop column `$sql_field_name` ";
}
+####################################################
+# Change the type of a column to the type defined in the schema
+####################################################
+
+sub change_column_field_type {
+ my ($self, $field_name) = @_;
+ return 0 unless defined $self->{record}->FIELD_DATA->{$field_name};
+ eval {
+ $self->dbh->do('ALTER TABLE `'
+ . $self->sql_table_name
+ . '` MODIFY '
+ . $self->sql_field_name($field_name) . ' '
+ . $self->{record}->FIELD_DATA->{$field_name}{type}
+ . ';');
+ };
+ return $@ ? 0 : 1;
+}
+
####################################################
# rebuild indexes for the table
####################################################
@@ -375,9 +393,8 @@ sub _drop_column_field_stmt {
sub rebuild_indexes {
my ($self) = @_;
- my $sql_table_name = $self->sql_table_name;
- my $field_data = $self->field_data;
- my %override_fields = reverse %{ $self->{params}{fieldOverride} };
+ my $sql_table_name = $self->sql_table_name;
+ my $field_data = $self->field_data;
# A key field column is going to be removed. The schema will not have the information for this column. So the
# indexes need to be obtained from the database. Note that each element of the returned array is an array reference
@@ -398,7 +415,7 @@ sub rebuild_indexes {
my $column = (grep { $index->[4] eq $_->[0] } @$columns)[0];
if (defined $column && $column->[5] =~ m/AUTO_INCREMENT/i) {
$self->dbh->do("ALTER TABLE `$sql_table_name` MODIFY `$column->[0]` $column->[1]");
- push @auto_increment_fields, $override_fields{ $column->[0] } // $column->[0];
+ push @auto_increment_fields, $column->[0];
}
$self->dbh->do("ALTER TABLE `$sql_table_name` DROP INDEX `$index->[2]`");
diff --git a/lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm b/lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm
index 6f3e3f8a23..c91808dff0 100644
--- a/lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm
+++ b/lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm
@@ -97,8 +97,6 @@ sub where_user_id_eq_set_id_eq_problem_id_eq {
# FIXME the rest of the places in this class that generate field lists (basically
# anywhere that calls grok_*_from_vsetID_sql), should call this method instead.
-# this method can handle if the set_id field has a fieldOverride set for it, and
-# the other methods can't.
sub sql_field_expression {
my ($self, $field, $table) = @_;
diff --git a/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm b/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm
index 20f7760941..44d3e3b1e4 100644
--- a/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm
+++ b/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm
@@ -211,11 +211,9 @@ sub checkTableFields {
exists $db->{$table}{params}{tableOverride} ? $db->{$table}{params}{tableOverride} : $table;
warn "$table_name is a non native table" if $db->{$table}{params}{non_native};
my @schema_field_names = $db->{$table}{record}->FIELDS;
- my %schema_override_field_names;
+ my %schema_field_names = map { $_ => 1 } @schema_field_names;
for my $field (@schema_field_names) {
- my $field_name = $db->{$table}{params}{fieldOverride}{$field} || $field;
- $schema_override_field_names{$field_name} = $field;
- if ($db->{$table}->tableFieldExists($field_name)) {
+ if ($db->{$table}->tableFieldExists($field)) {
$fieldStatus{$field} = [SAME_IN_A_AND_B];
} else {
$fieldStatus{$field} = [ONLY_IN_A];
@@ -229,10 +227,19 @@ sub checkTableFields {
my %database_fields = map { ${$_}[0] => $_ } @$result; # Construct a hash of field names to field data.
for my $field_name (keys %database_fields) {
- unless (exists($schema_override_field_names{$field_name})) {
+ unless (exists($schema_field_names{$field_name})) {
$fields_ok = 0;
$fieldStatus{$field_name} = [ONLY_IN_B];
push(@{ $fieldStatus{$field_name} }, 1) if $database_fields{$field_name}[3];
+ } else {
+ my $data_type = $database_fields{$field_name}[1];
+ $data_type =~ s/\(\d*\)$// if $data_type =~ /^(big|small)?int\(\d*\)$/;
+ $data_type = lc($data_type);
+ my $schema_data_type = lc($db->{$table}{record}->FIELD_DATA->{$field_name}{type} =~ s/ .*$//r);
+ if ($data_type ne $schema_data_type) {
+ $fieldStatus{$field_name} = [DIFFER_IN_A_AND_B];
+ $fields_ok = 0;
+ }
}
}
@@ -249,7 +256,7 @@ the same as the ones specified by the databaseLayout
=cut
sub updateTableFields {
- my ($self, $courseName, $table, $delete_field_names) = @_;
+ my ($self, $courseName, $table, $delete_field_names, $fix_type_field_names) = @_;
my @messages;
# Fetch schema from course environment and search database for corresponding tables.
@@ -290,6 +297,24 @@ sub updateTableFields {
}
}
+ # Change types of fields list in $fix_type_field_names to the type defined in the schema.
+ for my $field_name (@$fix_type_field_names) {
+ if ($fieldStatus->{$field_name} && $fieldStatus->{$field_name}[0] == DIFFER_IN_A_AND_B) {
+ if ($schema_obj->can('change_column_field_type') && $schema_obj->change_column_field_type($field_name)) {
+ push(@messages, [ "Changed type of column '$field_name' from table '$table'", 1 ]);
+ } else {
+ push(
+ @messages,
+ [
+ "Failed to changed type of column '$field_name' from table '$table'. "
+ . 'It is recommended that you delete this course and restore it from an archive.',
+ 0
+ ]
+ );
+ }
+ }
+ }
+
return @messages;
}
diff --git a/lib/WeBWorK/Utils/CourseManagement.pm b/lib/WeBWorK/Utils/CourseManagement.pm
index 206fc98120..cb95b5eba6 100644
--- a/lib/WeBWorK/Utils/CourseManagement.pm
+++ b/lib/WeBWorK/Utils/CourseManagement.pm
@@ -1303,18 +1303,15 @@ sub initNonNativeTables {
#fields in the schema. Its not a huge issue if the database table has extra columns.
} else {
my %fieldStatus;
- my $fields_ok = 1;
- my @schema_field_names = $db->{$table}->{record}->FIELDS;
- my %schema_override_field_names = ();
- foreach my $field (sort @schema_field_names) {
- my $field_name = $db->{$table}->{params}->{fieldOverride}->{$field} || $field;
- $schema_override_field_names{$field_name} = $field;
+ my $fields_ok = 1;
+ my @schema_field_names = $db->{$table}->{record}->FIELDS;
+ foreach my $field_name (sort @schema_field_names) {
my $database_field_exists = $db->{$table}->tableFieldExists($field_name);
#if the field doesn't exist then try to add it...
if (!$database_field_exists) {
$fields_ok = 0;
- $fieldStatus{$field} = [ONLY_IN_A];
- warn "$field from $database_table_name (aka |$table|) is only in schema, "
+ $fieldStatus{$field_name} = [ONLY_IN_A];
+ warn "$field_name from $database_table_name (aka |$table|) is only in schema, "
. "not in database, so adding it ... ";
if ($db->{$table}->can("add_column_field")) {
if ($db->{$table}->add_column_field($field_name)) {
diff --git a/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep
index 9385d07be3..b70c2b4898 100644
--- a/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep
+++ b/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep
@@ -6,8 +6,14 @@
+ <%= maketext(
+ 'There are database fields that do not have the same type as the field defined in the schema for '
+ . 'at least one table. The types of these fields can be changed when upgrading the course. '
+ . 'Make sure that you archive the course here before doing that, since changing the type of a '
+ . 'field can corrupt the table.'
+ ) =%>
+
+% }
% if (!$all_tables_ok) {
<%= maketext('The course database must be upgraded before archiving this course.') =%>
diff --git a/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep
index e32033dc85..4c9322d704 100644
--- a/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep
+++ b/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep
@@ -6,8 +6,14 @@
+ <%= maketext(
+ 'There are database fields that do not have the same type as the field defined in the schema for '
+ . 'at least one table. The types of these fields can be changed when upgrading the course. '
+ . 'Make sure that you archive the course before doing that, since changing the type of a '
+ . 'field can corrupt the table.'
+ ) =%>
+
+% }
% if (!$all_tables_ok) {
<%= maketext('The course database must be upgraded before renaming this course.') =%>
diff --git a/templates/ContentGenerator/CourseAdmin/upgrade_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/upgrade_course_confirm.html.ep
index 3d5e4ca7d0..fd1ed16e61 100644
--- a/templates/ContentGenerator/CourseAdmin/upgrade_course_confirm.html.ep
+++ b/templates/ContentGenerator/CourseAdmin/upgrade_course_confirm.html.ep
@@ -19,6 +19,15 @@
% }
+ % if ($incorrect_type_database_fields_exist) {
+