Bug 36993

I was able to solve it by going into the Database and running the following commands.

-- Disable foreign key checks
SET FOREIGN_KEY_CHECKS=0;

-- Drop the existing foreign key constraint
ALTER TABLE aqbudgets DROP FOREIGN KEY aqbudgetperiods_ibfk_1;

-- Modify the column to set it as NOT NULL
ALTER TABLE aqbudgets MODIFY COLUMN budget_period_id INT NOT NULL;

-- Add the foreign key constraint back
ALTER TABLE aqbudgets ADD CONSTRAINT aqbudgetperiods_ibfk_1 FOREIGN KEY (budget_period_id) REFERENCES aqbudgetperiods(budget_period_id);

-- Enable foreign key checks
SET FOREIGN_KEY_CHECKS=1;

And then re-running the upgrade, which gave the following output:

Setting up koha-common (24.05.00-1) ...
Upgrading database schema for library
Upgrade to 23.12.00.023  [03:22:38]: Bug 32132 - Set aqbudgets.budget_period_id as NOT NULL
	Updated aqbudgets.budget_period_id to NOT accept NULL values
Upgrade to 23.12.00.024  [03:22:38]: Bug 35065 - Move ILLModuleCopyrightClearance to additional contents
	Removed system preference 'ILLModuleCopyrightClearance'
Upgrade to 23.12.00.025  [03:22:38]: Bug 33393 - Modify sentence above the order table in English 1-page order PDF
	Added system preference '1PageOrderPDFText'
Upgrade to 23.12.00.026  [03:22:38]: Bug 35616 - Add source column to tickets table
	Added column 'tickets.source'
Upgrade to 23.12.00.027  [03:22:38]: Bug 32602 - Add administrative plugins type
	Added new permission 'plugins_admin'
Upgrade to 23.12.00.028  [03:22:38]: Bug 16122 - Add localuse column to items table and deleted items table
	Added column 'items.localuse'
	Added column 'deleteditems.localuse'
	You may use the new /misc/maintenance/update_localuse_from_statistics.pl script to populate the new field from the existing statistics data
Upgrade to 23.12.00.029  [03:22:45]: Bug 35919 - Add record_sources table
	Added new table 'record_sources'
	Added new column 'biblio_metadata.record_source_id'
	Added new foreign key 'biblio_metadata.record_metadata_fk_2'
	Added new column 'deletedbiblio_metadata.record_source_id'
	Added new foreign key 'deletedbiblio_metadata.record_metadata_fk_2'
	Added new permission 'manage_record_sources'
Upgrade to 23.12.00.030  [03:22:45]: Bug 35728 - Add option to NOT redirect to result when search returns only one record
	Added system preference 'RedirectToSoleResult'
Upgrade to 23.12.00.031  [03:22:45]: Bug 31791 - Add the ability to lock record modification
	Added new permission 'editcatalogue.edit_locked_records'
	Added new permission 'editcatalogue.set_record_sources'
Upgrade to 23.12.00.032  [03:22:45]: Bug 33478 - Customise the format of notices when they are printed
	Added column 'letter.style'
Upgrade to 23.12.00.033  [03:22:45]: Bug 31652 - Add geo-search: new value for search_field.type enum
	Added new value 'geo_point' to search_field.type enum
Upgrade to 23.12.00.034  [03:22:45]: Bug 12802 - Change type of system preference EmailFieldPrimary to multiple
	Updated system preference 'EmailFieldPrimary' to include 'selected addresses' option
	Added new system preference 'EmailFieldSelection'
Upgrade to 23.12.00.035  [03:22:45]: Bug 29393 - Add permission borrowers:send_messages_to_borrowers
	Added new permission 'send_messages_to_borrowers'
Upgrade to 23.12.00.036  [03:22:45]: Bug 35138 - Make the Elasticsearch facets configurable
	Updated DisplayLibraryFacets and search field configuration
Upgrade to 23.12.00.037  [03:22:45]: Bug 35626 - Add statuses to catalog concerns
	Added column 'tickets.status'
	Added column 'ticket_updates.status'
	Added TICKET_STATUS authorised value category
Upgrade to 23.12.00.038  [03:22:45]: Bug 32435 - Add ticket resolutions to catalog concerns
	Added TICKET_RESOLUTION authorised value category
Upgrade to 23.12.00.039  [03:22:45]: Bug 36002 - Remove aqorders.purchaseordernumber
Upgrade to 23.12.00.040  [03:22:45]: Bug 32610 - Add option for additional patron attributes of type date
	Added column 'borrower_attribute_types.is_date'
Upgrade to 23.12.00.041  [03:22:45]: Bug 15565 - Add DisplayMultiItemHolds system preference
	Added new system preference 'DisplayMultiItemHolds'
Upgrade to 23.12.00.042  [03:22:45]: Bug 35657 - Add assignee_id to tickets
	Added column 'tickets.assignee_id'
	Added column 'ticket_updates.assignee_id'
Upgrade to 23.12.00.043  [03:22:45]: Bug 25159 - Add action logs should be stored in JSON ( and as a diff of the change )
	Added column 'action_logs.diff'
Upgrade to 23.12.00.044  [03:22:45]: Bug 36120 - Add pickup location to bookings
	Added column 'bookings.pickup_library_id'
	Set existing bookings pickup location to item homebranch
	Set pickup_library_id to NOT NULL
Upgrade to 23.12.00.045  [03:22:45]: Bug 22740 - Preferences to enable automated setting of lost status when the associated fine is paid or written off
	Added new system preference 'UpdateItemLostStatusWhenPaid'
	Added new system preference 'UpdateItemLostStatusWhenWriteoff'
Upgrade to 23.12.00.046  [03:22:45]: Bug 27753 - Automate resolution of return claim when checking in an item
	Added new system preference 'AutoClaimReturnStatusOnCheckin'
	Added new system preference 'AutoClaimReturnStatusOnCheckout'
Upgrade to 23.12.00.047  [03:22:45]: Bug 35169 - Add new system preferences for longoverdue.pl borrowers categories
	Added new system preference 'DefaultLongOverduePatronCategories'
	Added new system preference 'DefaultLongOverdueSkipPatronCategories'
Upgrade to 23.12.00.048  [03:22:45]: Bug 33134 - Add some 76 missing languages
	Added 76 new languages
Upgrade to 23.12.00.049  [03:22:45]: Bug 36396 - Link Elasticsearch facets with authorized value categories
	Added Elasticsearch facets with authorized value categories
Upgrade to 23.12.00.050  [03:22:45]: Bug 36687 - Set itemtypes.notforloan to NOT NULL and tinyint(1)
	Updated itemtypes.notforlaon column'
Upgrade to 23.12.00.051  [03:22:45]: Bug 31097 - Change default 'Manual' restriction test
	Updated patron restriction types display for MANUAL restrictions from 'Manual' to 'Manual restriction'
Upgrade to 23.12.00.052  [03:22:46]: Bug 32256 - Self checkout batch mode
	Added system preference 'SCOBatchCheckoutsValidCategories'
Upgrade to 23.12.00.053  [03:22:46]: Bug 36755 - Increase length of 'code' column in borrower_attribute_types
	Increased borrower_attribute_types.code column length from 10 to 64
Upgrade to 23.12.00.054  [03:22:46]: Bug 19768 - Add 'Title notes' tab to OpacSerialDefaultTab system preference
	Added 'Title notes' option to opacSerialDefaultTab system preference
Upgrade to 23.12.00.055  [03:22:46]: Bug 30047 - Add thesaurus and heading fields to auth_header table
	Added column 'auth_header.heading'
Upgrade to 23.12.00.056  [03:22:46]: Bug 35149 - Add 'do nothing' option to CircAutoPrintQuickSlip syspref explanation
	Updated system preference 'CircAutoPrintQuickSlip'
Upgrade to 23.12.00.057  [03:22:46]: Bug 29948 - Display author information for researchers
	Added new system preference 'OPACAuthorIdentifiersAndInformation'
	  Removed system preference 'OPACAuthorIdentifiers'
Upgrade to 23.12.00.058  [03:22:46]: Bug 28869 - Add authorised_value_categories.is_integer_only
Upgrade to 23.12.00.059  [03:22:46]: Bug 36819 - Change barcode width value if it still has the wrong default value
	Changed the barcode width in patron card creator default layout from 8% to 80%.
Upgrade to 23.12.00.060  [03:22:46]: Bug 36665 - Add system preference StaffLoginBranchBasedOnIP
	Added new system preference 'StaffLoginBranchBasedOnIP'
Upgrade to 23.12.00.061  [03:22:46]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
	Renamed system preference 'AutoLocation' to 'StaffLoginRestrictLibraryByIP'
Upgrade to 24.05.00.000  [03:22:46]: Koha 24.05.00 release

(In reply to Matthew Charlesworth from comment #1)
> I was able to solve it by going into the Database and running the following
> commands.
> 
> -- Disable foreign key checks
> SET FOREIGN_KEY_CHECKS=0;
> 
> -- Drop the existing foreign key constraint
> ALTER TABLE aqbudgets DROP FOREIGN KEY aqbudgetperiods_ibfk_1;
> 
> -- Modify the column to set it as NOT NULL
> ALTER TABLE aqbudgets MODIFY COLUMN budget_period_id INT NOT NULL;
> 
> -- Add the foreign key constraint back
> ALTER TABLE aqbudgets ADD CONSTRAINT aqbudgetperiods_ibfk_1 FOREIGN KEY
> (budget_period_id) REFERENCES aqbudgetperiods(budget_period_id);
> 
> -- Enable foreign key checks
> SET FOREIGN_KEY_CHECKS=1;
> 

It's great that you were able to solve it, but I will re-open this bug as it's been reported a few times now and I think we need to make sure that we solve it in Koha properly.

Matthew, since this might play a role in it working for some but not others: What is the DBMS you are using and the exact version of it?

*** Bug 36992 has been marked as a duplicate of this bug. ***

Hi Katrin,

(In reply to Katrin Fischer from comment #3)
> Matthew, since this might play a role in it working for some but not others:
> What is the DBMS you are using and the exact version of it?

Here is the Version information from the Server Information tab in the About Koha page.

Koha version: 24.05.00.000
OS version ('uname -a'): Linux ip-172-31-36-26 5.15.0-107-generic #117-Ubuntu SMP Fri Apr 26 12:26:49 UTC 2024 x86_64
Perl version: 5.034000
MySQL version: 5.5.5-10.6.16-MariaDB-0ubuntu0.22.04.1
Apache version: Server version: Apache/2.4.52 (Ubuntu)

M.

I'm unable to reproduce this on my environment, but this might be DBMS version issue as Katrin suggested on bug 32132. I think proper way to fix this is to drop key, make changes to the field and then adding key again as Matthew did to solve this. This has been done also in similar bugs (see bug 31086).

Katrin, do I make changes to the existing db_revs file (231200023.pl) or should I make a new one?

It's a good question. I think you are correct that we need to fix the database update itself so people don't get stuck on it. Thx for the quick reaction!

(In reply to Matthew Charlesworth from comment #5)
> Hi Katrin,
> 
> (In reply to Katrin Fischer from comment #3)
> > Matthew, since this might play a role in it working for some but not others:
> > What is the DBMS you are using and the exact version of it?
> 
> Here is the Version information from the Server Information tab in the About
> Koha page.
> 
> Koha version: 24.05.00.000
> OS version ('uname -a'): Linux ip-172-31-36-26 5.15.0-107-generic
> #117-Ubuntu SMP Fri Apr 26 12:26:49 UTC 2024 x86_64
> Perl version: 5.034000
> MySQL version: 5.5.5-10.6.16-MariaDB-0ubuntu0.22.04.1
> Apache version: Server version: Apache/2.4.52 (Ubuntu)
> 
> M.

Well that's odd, I have almost exactly same versions of each of these. Except OS is different.

(In reply to Emmi Takkinen from comment #8)
> (In reply to Matthew Charlesworth from comment #5)
> > Hi Katrin,
> > 
> > (In reply to Katrin Fischer from comment #3)
> > > Matthew, since this might play a role in it working for some but not others:
> > > What is the DBMS you are using and the exact version of it?
> > 
> > Here is the Version information from the Server Information tab in the About
> > Koha page.
> > 
> > Koha version: 24.05.00.000
> > OS version ('uname -a'): Linux ip-172-31-36-26 5.15.0-107-generic
> > #117-Ubuntu SMP Fri Apr 26 12:26:49 UTC 2024 x86_64
> > Perl version: 5.034000
> > MySQL version: 5.5.5-10.6.16-MariaDB-0ubuntu0.22.04.1
> > Apache version: Server version: Apache/2.4.52 (Ubuntu)
> > 
> > M.
> 
> Well that's odd, I have almost exactly same versions of each of these.
> Except OS is different.

Could it be something like the sql strict settings?

Dear Katrin,

(In reply to Katrin Fischer from comment #9)
> Could it be something like the sql strict settings?

MariaDB [...]> SHOW VARIABLES LIKE 'sql_mode';
+---------------+-------------------------------------------------------------------------------------------+
| Variable_name | Value                                                                                     |
+---------------+-------------------------------------------------------------------------------------------+
| sql_mode      | STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION |
+---------------+-------------------------------------------------------------------------------------------+
1 row in set (0.004 sec)

M.

Thanks!

Emmi: I think we should still do what you suggest and drop/recreate - that should put us on the save side. 

...and maybe as this happened before a coding guideline?

(In reply to Katrin Fischer from comment #11)
> Thanks!
> 
> Emmi: I think we should still do what you suggest and drop/recreate - that
> should put us on the save side. 
> 
> ...and maybe as this happened before a coding guideline?

I think so too, I'll take a look if there is an example how we have dealt with these kind of situation previously and then provide a patch.

By the way, I just checked what my database says when "SHOW VARIABLES LIKE 'sql_mode';" is run and result is same as Matthews :D

Fixing existing db_rev file seems to be the way to go (bug 34516).

I just realized that there might be situations where aqbudgets.budget_period_id has value of non-existing budget_period_id, so this has to be fixed also. Otherwise adding "aqbudgetperiods_ibfk_1" doesn't work.

Matthew: could it be that you have values in aqbudgets.budget_period_id column that don't exist on aqbudgetperiods.budget_period_id?

Hi Emmi,

(In reply to Emmi Takkinen from comment #15)
> Matthew: could it be that you have values in aqbudgets.budget_period_id
> column that don't exist on aqbudgetperiods.budget_period_id?

To be honest I'm not sure. If I do this, does this tell you?

MariaDB [...]> select budget_period_id from aqbudgets;
+------------------+
| budget_period_id |
+------------------+
|                1 |
+------------------+
1 row in set (0.001 sec)

MariaDB [...]> select budget_period_id from aqbudgetperiods;
+------------------+
| budget_period_id |
+------------------+
|                1 |
+------------------+
1 row in set (0.000 sec)

It looks like they're the same.

M.

(In reply to Matthew Charlesworth from comment #16)
> Hi Emmi,
> 
> (In reply to Emmi Takkinen from comment #15)
> > Matthew: could it be that you have values in aqbudgets.budget_period_id
> > column that don't exist on aqbudgetperiods.budget_period_id?
> 
> To be honest I'm not sure. If I do this, does this tell you?
> 
> MariaDB [...]> select budget_period_id from aqbudgets;
> +------------------+
> | budget_period_id |
> +------------------+
> |                1 |
> +------------------+
> 1 row in set (0.001 sec)
> 
> MariaDB [...]> select budget_period_id from aqbudgetperiods;
> +------------------+
> | budget_period_id |
> +------------------+
> |                1 |
> +------------------+
> 1 row in set (0.000 sec)
> 
> It looks like they're the same.
> 
> M.

Hmm, they are indeed. This should also return empty:

SELECT COUNT(*) FROM aqbudgets WHERE budget_period_id IS NULL OR budget_period_id NOT IN(SELECT budget_period_id FROM aqbudgetperiods);

Hi,

(In reply to Emmi Takkinen from comment #17)
> Hmm, they are indeed. This should also return empty:
> 
> SELECT COUNT(*) FROM aqbudgets WHERE budget_period_id IS NULL OR
> budget_period_id NOT IN(SELECT budget_period_id FROM aqbudgetperiods);

Well, 0...

MariaDB [...]> SELECT COUNT(*) FROM aqbudgets WHERE budget_period_id IS NULL OR budget_period_id NOT IN(SELECT budget_period_id FROM aqbudgetperiods);
+----------+
| COUNT(*) |
+----------+
|        0 |
+----------+
1 row in set (0.000 sec)

M.

Created attachment 167306 [details] [review]
Bug 36993: Upgrade fails at 23.12.00.023 [Bug 32132]

Database update fails on some databases because of
foreign key constraint error. This comes from attempt
to make column aqbudgets.budget_period_id not accept
NULL values. Update also fails if there are rows where
column aqbudgets.budget_period_id doesn't match any
values in aqbudgetperiods.budget_period_id.

To test:
1. Remove changes made in bug 32132 and downgrade your database:
- ALTER TABLE aqbudgets MODIFY COLUMN `budget_period_id` INT(11) NULL;
- UPDATE aqbudgets SET budget_period_id = NULL
WHERE budget_period_id IN(SELECT budget_period_id FROM aqbudgetperiods
WHERE budget_period_description = "Budget for funds without budget");
- DELETE FROM aqbudgetperiods
WHERE budget_period_description = "Budget for funds without budget";
- UPDATE systempreferences SET value="23.1200022" WHERE variable = "Version;
2. Upgrade your database (e.g. running installer/data/mysql/updatedatabase.pl)
=> Update fails on foreign key constraint error.
4. Apply this patch.
5. Try to update your database again.
=> Database should now be upgraded succesfully.

Created attachment 167315 [details]
Bug 36993 - Output from testing

I had a go at testing, but I couldn't the upgrade to fail for aqbudgets - it does fail for a different bug (bug 26176).

I've attached a file with the output from the different commands if this is of any help.

Testing using KTD (which uses MariaDB I think).

(In reply to David Nind from comment #20)
> Created attachment 167315 [details]
> Bug 36993 - Output from testing
> 
> I had a go at testing, but I couldn't the upgrade to fail for aqbudgets - it
> does fail for a different bug (bug 26176).
> 
> I've attached a file with the output from the different commands if this is
> of any help.
> 
> Testing using KTD (which uses MariaDB I think).

I think we still haven't managed to find the very source of the issue (DBMS version, configuration etc.). If the database update still passes and does the requires change, that might also be ok.

Created attachment 167377 [details] [review]
Bug 36993: Upgrade fails at 23.12.00.023 [Bug 32132]

Database update fails on some databases because of
foreign key constraint error. This comes from attempt
to make column aqbudgets.budget_period_id not accept
NULL values. Update also fails if there are rows where
column aqbudgets.budget_period_id doesn't match any
values in aqbudgetperiods.budget_period_id.

To test:
1. Remove changes made in bug 32132 and downgrade your database:
- ALTER TABLE aqbudgets MODIFY COLUMN `budget_period_id` INT(11) NULL;
- UPDATE aqbudgets SET budget_period_id = NULL
WHERE budget_period_id IN(SELECT budget_period_id FROM aqbudgetperiods
WHERE budget_period_description = "Budget for funds without budget");
- DELETE FROM aqbudgetperiods
WHERE budget_period_description = "Budget for funds without budget";
- UPDATE systempreferences SET value="23.1200022" WHERE variable = "Version;
2. Upgrade your database (e.g. running installer/data/mysql/updatedatabase.pl)
=> Update fails on foreign key constraint error.
4. Apply this patch.
5. Try to update your database again.
=> Database should now be upgraded succesfully.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

I'm pretty confident this won't cause any further issues and makes the update more reliable.. I think we should push it and see ;P

Passing QA

Pushed to main for 24.11.00, thanks all.

Pushed to 24.05.x for 24.05.01

Not for 23.11.x

(In reply to Katrin Fischer from comment #21)
> I think we still haven't managed to find the very source of the issue (DBMS
> version, configuration etc.). If the database update still passes and does
> the requires change, that might also be ok.

It most likely is the sql strict settings - see bug 35687.

And +1 for adding a coding guideline! Maybe we can discuss at next week's dev meeting?

(In reply to Emily Lamancusa from comment #27)
> (In reply to Katrin Fischer from comment #21)
> > I think we still haven't managed to find the very source of the issue (DBMS
> > version, configuration etc.). If the database update still passes and does
> > the requires change, that might also be ok.
> 
> It most likely is the sql strict settings - see bug 35687.
> 
> And +1 for adding a coding guideline! Maybe we can discuss at next week's
> dev meeting?

Just followed up on this and it looks like we decided to add one :)
https://hedgedoc.ishukone.net/Development_IRC_meeting_12_June_2024#