pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
298 stars 443 forks source link

Upgrade 3.2 to 3.3-LTS fails due "Null value not allowed" for review_assigments.review_round_id #10031

Open marcbria opened 1 month ago

marcbria commented 1 month ago

Describe the bug When trying to upgrade to the last LTS (OJS 3.3.0-17) from old OJS 3.2 (that comes from 2.x with 20 years old data) I got the error:

ERROR: Upgrade failed: DB: SQLSTATE[22004]: Null value not allowed: 1138 Invalid use of NULL value (SQL: ALTER TABLE review_assignments CHANGE review_round_id review_round_id BIGINT NOT NULL)

When I check the DB I found 166 ancient rows (2007-2008) from the 3599, don't have a value for review_round_id.

When I assign those nulls to "0" the upgrade continued without no further complaints... but I'm not sure if there will be any collateral damage for this workaround.

If it's a solution, should the upgrade script include this conversion too or you feel it's is something too wired?

asmecher commented 1 month ago

@jonasraoni or @mfelczak, do you know if hosting has run into this issue?

jonasraoni commented 1 month ago

I just helped to solve a couple of issues, so I haven't seen this one. The most experienced OJS 2 > 3 upgrader on the house is @jnugent 😁

jnugent commented 1 month ago

@jonasraoni @asmecher @marcbria It's possible there are old review rounds in your database that belong to submissions that no longer exist. We occasionally run into a bug where the review_assignments table has a null review_round_id and the way we fix this is with an SQL query like:

UPDATE review_assignments, review_rounds SET review_assignments.review_round_id = review_rounds.review_round_id WHERE review_assignments.submission_id = review_rounds.submission_id AND review_assignments.round = review_rounds.round;

Or something like that (test it first!). This won't fix stale database records that reference submissions that don't exist any more so perhaps some data clean-up is advised here as upgrading to 3.4 may be problematic later on if they are left hanging around.

jonasraoni commented 1 month ago

If the problem exists, I think it must be fixed, the upgrade path for old users is already not simple with the "pit-stop" step.

marcbria commented 1 month ago

Thanks for your help guys!

@jnugent it's safe to run this in any journal? I can add this to the bottom of each mysql dump... just in case the journal fails for this reason.

If it's safe, make sense to add this in the upgradeScript? Could be reported in log as "Removing orphan reviews for article XXX that don't exist any more" or something like that.

Healing old-damaged DBs during upgrades is a present for our future selves. :-).

jnugent commented 1 month ago

@marcbria I'm pretty cautious and would normally only run this after a database backup, please. If you're doing that you should be alright.

marcbria commented 1 month ago

Of course. I work on exact replicas of my journals and never touch anything in production until I'm absolutely sure it works in testing. :-)

Right now I'm doing the upgrade of 50 journals all our rubbish is coming to light. :-)

The point is, in other journals, I have found a couple of "DB integrity" issues more but related with authors and email_logs tables. Should I post them in the forum, in this thread or create an issue for each of them?

If it's safe, make sense to add this in the upgradeScript? It could be reported in log as "Removing orphan reviews for article XXX that don't exist any more" or something like that.

I meant if you think this is something to include in the OJS upgrade script in case others run into this error in the future.

Probably the answer should be "no" as far looks like I'm the only one reporting this?

jonasraoni commented 1 month ago

Wherever you post them, I'm interested to know more 😁 Even though they are painful, we've got to make the way from 2.x to 3.x as clear as possible.

jonasraoni commented 3 weeks ago

@marcbria Are you able to share an installation that had this problem in private? I'm interested to get the problem fixed.

jonasraoni commented 2 weeks ago

I've got a database to inspect, so I've assigned myself.