Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the timestamp column in the past answer table to BIGINT. #2648

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

drgrice1
Copy link
Member

Currently this column is of INT type. The INT type is a 4 bit integer, and so a timestamp stored in this column that is after the year 2038 will not work. A BIGINT is large enough for my lifetime at least (and then another 292 billion years or so).

There is no need to update databases since the INT and BIGINT types are compatible. However, at some point you will need to make sure that the type of the column in all courses is updated. You can run the upgrade-database-to-utf8mb4.pl script to do this. For example, running upgrade-database-to-utf8mb4.pl -c courseId will update the past answer table for the named course. Perhaps that script should be renamed? Although it does have special handling for converting the database to utf8mb4, it also generally makes sure that the database columns are the same as specified in the webwork2 schema. Or perhaps that part of the script should be separated into another script?

This fixes issue #952.

@drgrice1
Copy link
Member Author

Note I have also carefully reviewed all database columns that hold timestamps, and all of the rest are already of type BIGINT.

Currently this column is of `INT` type.  The `INT` type is a 4 bit
integer, and so a timestamp stored in this column that is after the year
2038 will not work.  A `BIGINT` is large enough for my lifetime at least
(and then another 292 billion years or so).

There is no need to update databases since the INT and BIGINT types are
compatible.  However, at some point you will need to make sure that the
type of the column in all courses is updated.  You can run the
`upgrade-database-to-utf8mb4.pl` script to do this. For example, running
`upgrade-database-to-utf8mb4.pl -c courseId` will update the past answer
table for the named course.

This fixes issue openwebwork#952.
@somiaj
Copy link
Contributor

somiaj commented Dec 18, 2024

Is it possible for the upgrade course action on the course admin page to do this update without manually running that script?

@drgrice1
Copy link
Member Author

Funny that you ask. I am actually toying with integrating what the script does into the update check as we speak!

I am certain that it can be done, but should it just do the type change for those that differ, or should it instead be an option when upgrading a course. This is sort of like fields that exist in the database, but are not in the schema. There are check boxes you can check to delete those fields, or you can leave the fields in the database since they don't matter. This is similar in that types can differ (at least in some cases) and the field will still work. Although, there are limitations on how types can differ. If the field has INT type, but the schema has BIGINT type, then it will work as long as the integers stored in the column do not exceed the INT size. The other way would be perfectly fine though. Clearly if the field has INT type and the schema has TEXT type, that wouldn't work.

It is certainly easier to implement if you just make the upgrade process change the field type without an option.

@drgrice1
Copy link
Member Author

There are probably also limitations on being able to do a type change. The example of an INT versus TEXT type may not even work to perform the type change.

@drgrice1
Copy link
Member Author

I am also not sure it is a good idea to implement this in the course upgrade process. Failures can occur in this process, and those failures can leave the tables for the course in an unmanageable state. The script has the advantage of making a database backup of the entire database that can be restored if something goes wrong. I have a course that the script fails on in fact. The course's password table still has the type of that field as a tinyblob, and when the script tries to convert the field to a varchar(100) it fails with a warning that there is a duplicate entry for the key 'user_id'.

@somiaj
Copy link
Contributor

somiaj commented Dec 18, 2024

I hadn't considered this in general, maybe if anything were to be added to the upgrade course it should target specific fields that we know can be easily changed like this case, vs a general tool that notices a change and tries to implement it.

I don't think this will be much of an issue, 2038 is a ways off, I suspect most courses will have been recreated a few times by then. If you say create a new course as a copy of an old course, the database would be updated appropriately? What about if you create a new course as a copy from an old course archive?

@drgrice1
Copy link
Member Author

I am not sure that the database would be updated in the case of creating a copy of course. I would have to check how that is done again.

@drgrice1 drgrice1 force-pushed the fix-past-answer-year-2038-bug branch from fe46bce to a40c26a Compare December 19, 2024 02:22
@somiaj
Copy link
Contributor

somiaj commented Dec 19, 2024

I tested what happens when a new course is copied to another course from add course. Here it appears a new course is created and data is just copied. The newly created copy had timestamp | bigint(20) set in my database for that row. I think for most users new copies will be safe.

@drgrice1
Copy link
Member Author

Any of the approaches that use the addCourse method from WeBWorK::Utils::CourseManagement will always be created according to what is in the schema. That includes anytime you add a course from the "Add Course" page in the admin course. That never truly copies a course as you say. It only copies components from a course. The database for the new course is constructed from scratch, and data from other course's tables are copied.

However, if one unarchives a course, deletes users, and then uses that course for a new course, the database is literally used. In that case the database columns will have the same types as the archived course. This is true even if you rename the course when you unarchive it. In this case it is just renaming tables.

@somiaj somiaj merged commit f36d1c5 into openwebwork:develop Dec 19, 2024
2 checks passed
@drgrice1 drgrice1 deleted the fix-past-answer-year-2038-bug branch December 19, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants