Description
Nick Clemens (kidclamp)
2020-11-06 11:41:20 UTC
Created attachment 113603 [details] [review] Bug 26947: Add scheam-only option to koha-dump This patch adds a new --schema-only optoin to the koha-dump script. This can be used to generate a new kohastructure.sql file during the release, and later to compare the DB structure of a koha instance to the kohastructure.sql file shipped with a version to identify any missing constraints or other db structure issues To test: 1 - debian/scripts/koha-dump kohadev 2 - Confirm db and configs are dumped correctly 3 - Apply patch 4 - debian/scripts/koha-dump --help 5 - Confirm new option is listed and makes sense 6 - debian/scripts/koha-dump --schema-only kohadev 7 - Confirm only schema is dumped and is not zipped 8 - debian/scripts/koha-dump kohadev 9 - Confirm entire db is dumped as before On koha-testing-docker: root@kohadevbox:koha(master)$ perl debian/scripts/koha-dump --schema-only kohadev Dumping Koha site kohadev: * schema to /var/spool/koha/kohadev/kohadev-schema-2020-11-17.sql mysqldump: Error: 'Access denied; you need (at least one of) the PROCESS privilege(s) for this operation' when trying to dump tablespaces Should not we have a bash function in ktd instead? Or a dev script in misc/devel? (In reply to Jonathan Druart from comment #2) > On koha-testing-docker: > > root@kohadevbox:koha(master)$ perl debian/scripts/koha-dump --schema-only > kohadev > Dumping Koha site kohadev: > * schema to /var/spool/koha/kohadev/kohadev-schema-2020-11-17.sql > mysqldump: Error: 'Access denied; you need (at least one of) the PROCESS > privilege(s) for this operation' when trying to dump tablespaces I was using MySQL8 and had to > GRANT USAGE, PROCESS ON *.* TO `koha_kohadev`@`%`; (In reply to Jonathan Druart from comment #2) > Should not we have a bash function in ktd instead? Or a dev script in > misc/devel? function update_kohastructure { mysqldump --databases -d --host=db --user=koha_kohadev --password=password koha_kohadev > installer/data/mysql/kohastructure.sql } C4::Installer::load_sql returned the following errors while attempting to load /kohadevbox/koha/installer/data/mysql/kohastructure.sql: DBD::mysql::st execute failed: Failed to open the referenced table 'branches' at /usr/share/perl5/DBIx/RunSQL.pm line 273. Something went wrong loading file /kohadevbox/koha/installer/data/mysql/kohastructure.sql ([SQL ERROR]: CREATE TABLE `account_credit_types_branches` ( Did it work for you? (In reply to Jonathan Druart from comment #5) > C4::Installer::load_sql returned the following errors while attempting to > load /kohadevbox/koha/installer/data/mysql/kohastructure.sql: > DBD::mysql::st execute failed: Failed to open the referenced table > 'branches' at /usr/share/perl5/DBIx/RunSQL.pm line 273. > Something went wrong loading file > /kohadevbox/koha/installer/data/mysql/kohastructure.sql ([SQL ERROR]: CREATE > TABLE `account_credit_types_branches` ( > > Did it work for you? Actually, the following works: > DROP DATABASE koha_kohadev > CREATE DATABASE koha_kohadev koha-mysql kohadev < installer/data/mysql/kohastructure.sql But reset_all does not, neither the installer (it says "Database tables created", but it's wrong...) My guess is that the following instruction line, from kohastructure.sql, is not read by DBIx::RunSQL 14 /*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */; we load the schema from C4::Installer 282 my $error = $self->load_sql("$datadir/kohastructure.sql") Something along the line 'sed -r --in-place 's/,\s*-- (.*)$/ COMMENT "\1",/g;' installer/data/mysql/kohastructure.sql' should be used before the first run of this to capture our existing `, -- this is a comment` type comments and convert them to `COMMENT 'this is a comment',` so we keep the data and it also gets added to DBIC schemas as per the research undertaken by Victor: https://wiki.koha-community.org/wiki/User:Victor_Grousset_-_tuxayo/Coding_Guidelines-SQL11-changes I didn't find a way to export the schema in an order different than alpha. I may have missed something somewhere... but I read the mysqldump man page (as well as mydumper) and didn't find anything. I guess it's not possible as you could have two tables having each one relation to the other. Created attachment 113794 [details] [review] Bug 26947: (follow-up) Set key checks to disabled for kohastructure Not entirely sure I like this approach.. it's very MySQL centric.. even MariaDB complains about some of the statements :(.. we can probably safely drop those, but I wasn't 100% sure what they did or why they appear in the dumpfile in the first place. The alternative is to fallback to using the system MySQL cli from the perl script.. but I don't really like that idea much either... so perhaps just falling back to disabling foreign key checks is the right thing to do here. Should not we only need to turn off FOREIGN_KEY_CHECKS? (In reply to Jonathan Druart from comment #12) > Should not we only need to turn off FOREIGN_KEY_CHECKS? I think you're probably right, I wasn't really sure why mysqldump turns so much else off.. I aim to try and understand that today and will quickly followup with details if I find any and likely re-submit that final patch with just the disablement of keys rather than all the other oddities. Created attachment 113799 [details] [review] Bug 26947: Move -- comment to the COMMENT clause Sed command from bug 26947 comment 8 sed -r --in-place 's/,\s*-- (.*)$/ COMMENT "\1",/g;' installer/data/mysql/kohastructure.sql Created attachment 113800 [details] [review] Bug 26947: Replace double quotes from comments Martin, I wanted to try your sed command, it works pretty well! I only had to adjust manually 3 occurrences. Attaching the patches here but feel free to drop them if needed. Created attachment 113802 [details] [review] Bug 26947: Add scheam-only option to koha-dump This patch adds a new --schema-only optoin to the koha-dump script. This can be used to generate a new kohastructure.sql file during the release, and later to compare the DB structure of a koha instance to the kohastructure.sql file shipped with a version to identify any missing constraints or other db structure issues To test: 1 - debian/scripts/koha-dump kohadev 2 - Confirm db and configs are dumped correctly 3 - Apply patch 4 - debian/scripts/koha-dump --help 5 - Confirm new option is listed and makes sense 6 - debian/scripts/koha-dump --schema-only kohadev 7 - Confirm only schema is dumped and is not zipped 8 - debian/scripts/koha-dump kohadev 9 - Confirm entire db is dumped as before Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 113803 [details] [review] Bug 26947: (follow-up) Set key checks to disabled for kohastructure This patch adds sql calls to disable foreign key checks for the initial kohastructure schema load and then re-enables them again for subsequent actions. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 113804 [details] [review] Bug 26947: Move -- comment to the COMMENT clause Sed command from bug 26947 comment 8 sed -r --in-place 's/,\s*-- (.*)$/ COMMENT "\1",/g;' installer/data/mysql/kohastructure.sql Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 113805 [details] [review] Bug 26947: Replace double quotes from comments Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> (In reply to Jonathan Druart from comment #4) > (In reply to Jonathan Druart from comment #2) > > Should not we have a bash function in ktd instead? Or a dev script in > > misc/devel? > > function update_kohastructure { > mysqldump --databases -d --host=db --user=koha_kohadev > --password=password koha_kohadev > installer/data/mysql/kohastructure.sql > } This one is better, but still not perfect: mysqldump --no-create-db --no-data --host=db --user=koha_kohadev --password=password koha_kohadev > installer/data/mysql/kohastructure.sql With the patch, the dump doesn't have the following part, is that expected? -- Current Database: `koha_kohadev` -- CREATE DATABASE /*!32312 IF NOT EXISTS*/ `koha_kohadev` /*!40100 DEFAULT CHARACTER SET latin1 */; USE `koha_kohadev`; -- (In reply to Victor Grousset/tuxayo from comment #22) > With the patch, the dump doesn't have the following part, is that expected? > > -- Current Database: `koha_kohadev` > -- > > CREATE DATABASE /*!32312 IF NOT EXISTS*/ `koha_kohadev` /*!40100 DEFAULT > CHARACTER SET latin1 */; > > USE `koha_kohadev`; > > -- Yes. Nick, Martin, what is your opinion about koha-dump vs the bash alias? Or even a simple bash script in misc/devel that would generate it? I think the following line is more simple and maintainable than tweaking koha-dump mysqldump --no-create-db --no-data --host=db --user=koha_kohadev --password=password koha_kohadev (In reply to Jonathan Druart from comment #24) > Nick, Martin, what is your opinion about koha-dump vs the bash alias? > > Or even a simple bash script in misc/devel that would generate it? > > I think the following line is more simple and maintainable than tweaking > koha-dump > > mysqldump --no-create-db --no-data --host=db --user=koha_kohadev > --password=password koha_kohadev The reason I like koha-dump is that it allows for easy dumping of the existing database for comparison - it provides a tool that is the same used during releasing An alias/script can do the same, but isn't quite as easy I think. What is the argument against koha-dump? (In reply to Nick Clemens from comment #25) > An alias/script can do the same, but isn't quite as easy I think. What is > the argument against koha-dump? 43 insertions(+), 26 deletions(-) in your patch, vs a 1 line change if we have an alias. I am ok with your changes if you think it will be easier as it. And also make a 3rd place for RM tools (we already have the release-tools repo and the misc/devel dir). Created attachment 113989 [details] [review] Bug 26947: Correct path for chown and chmod Created attachment 114021 [details] [review] Bug 26947: Add scheam-only option to koha-dump This patch adds a new --schema-only optoin to the koha-dump script. This can be used to generate a new kohastructure.sql file during the release, and later to compare the DB structure of a koha instance to the kohastructure.sql file shipped with a version to identify any missing constraints or other db structure issues To test: 1 - debian/scripts/koha-dump kohadev 2 - Confirm db and configs are dumped correctly 3 - Apply patch 4 - debian/scripts/koha-dump --help 5 - Confirm new option is listed and makes sense 6 - debian/scripts/koha-dump --schema-only kohadev 7 - Confirm only schema is dumped and is not zipped 8 - debian/scripts/koha-dump kohadev 9 - Confirm entire db is dumped as before Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 114022 [details] [review] Bug 26947: (follow-up) Set key checks to disabled for kohastructure This patch adds sql calls to disable foreign key checks for the initial kohastructure schema load and then re-enables them again for subsequent actions. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 114023 [details] [review] Bug 26947: Move -- comment to the COMMENT clause Sed command from bug 26947 comment 8 sed -r --in-place 's/,\s*-- (.*)$/ COMMENT "\1",/g;' installer/data/mysql/kohastructure.sql Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 114024 [details] [review] Bug 26947: Replace double quotes from comments Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 114025 [details] [review] Bug 26947: Correct path for chown and chmod Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> As an help for QA, here is another signoff. QA script ok Note not directly related to this ticket: kohadev-XXXX-XX-XX.tar.gz, the file dump, doesn't have the right unix group. And both .gz files (file dump and db dump) don't have the right group once extracted. Not sure if that's an issue. Created attachment 114195 [details] [review] Bug 26947: Add scheam-only option to koha-dump This patch adds a new --schema-only optoin to the koha-dump script. This can be used to generate a new kohastructure.sql file during the release, and later to compare the DB structure of a koha instance to the kohastructure.sql file shipped with a version to identify any missing constraints or other db structure issues To test: 1 - debian/scripts/koha-dump kohadev 2 - Confirm db and configs are dumped correctly 3 - Apply patch 4 - debian/scripts/koha-dump --help 5 - Confirm new option is listed and makes sense 6 - debian/scripts/koha-dump --schema-only kohadev 7 - Confirm only schema is dumped and is not zipped 8 - debian/scripts/koha-dump kohadev 9 - Confirm entire db is dumped as before Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 114196 [details] [review] Bug 26947: (follow-up) Set key checks to disabled for kohastructure This patch adds sql calls to disable foreign key checks for the initial kohastructure schema load and then re-enables them again for subsequent actions. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 114197 [details] [review] Bug 26947: Move -- comment to the COMMENT clause Sed command from bug 26947 comment 8 sed -r --in-place 's/,\s*-- (.*)$/ COMMENT "\1",/g;' installer/data/mysql/kohastructure.sql Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 114198 [details] [review] Bug 26947: Replace double quotes from comments Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 114199 [details] [review] Bug 26947: Correct path for chown and chmod Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 114201 [details] [review] Bug 26947: Update kohastructure.sql I haven't followed this change line by line, but does this mean that developers will no longer have to update kohastructure.sql? We'll just have to provide atomic updates? (In reply to David Cook from comment #41) > I haven't followed this change line by line, but does this mean that > developers will no longer have to update kohastructure.sql? We'll just have > to provide atomic updates? Changes to kohastructure.sql are still needed. (In reply to Jonathan Druart from comment #42) > > Changes to kohastructure.sql are still needed. Ah, so is this change just about synchronizing kohastructure.sql with the output of a schema only koha-dump? Mostly just so RMs can do a sanity check? I think Nick explained quite well the original need in comment 0: > Currently the file is out of order, and the conventions for punctuation and > formatting are not standard. For instance, some numbers are quoted, some > tables names are in backticks, some foreign keys are listed as 'CONSTRAINT' > and some aren't > > The advantage to regenerating the file is that then users can dump their db > schema and easily diff it against the expected structure to see what might > be missing. > > Many DB updates simply don't add a new constraint if the existing data > violates this, and as we use more Koha Objects, these missing constraints > cause issues. So it's a need for both standardisation/homogeneity and maintenance. Created attachment 115065 [details] [review] Bug 26947: Add schema-only option to koha-dump This patch adds a new --schema-only optoin to the koha-dump script. This can be used to generate a new kohastructure.sql file during the release, and later to compare the DB structure of a koha instance to the kohastructure.sql file shipped with a version to identify any missing constraints or other db structure issues To test: 1 - debian/scripts/koha-dump kohadev 2 - Confirm db and configs are dumped correctly 3 - Apply patch 4 - debian/scripts/koha-dump --help 5 - Confirm new option is listed and makes sense 6 - debian/scripts/koha-dump --schema-only kohadev 7 - Confirm only schema is dumped and is not zipped 8 - debian/scripts/koha-dump kohadev 9 - Confirm entire db is dumped as before Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 115066 [details] [review] Bug 26947: (follow-up) Set key checks to disabled for kohastructure This patch adds sql calls to disable foreign key checks for the initial kohastructure schema load and then re-enables them again for subsequent actions. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 115067 [details] [review] Bug 26947: Move -- comment to the COMMENT clause Sed command from bug 26947 comment 8 sed -r --in-place 's/,\s*-- (.*)$/ COMMENT "\1",/g;' installer/data/mysql/kohastructure.sql Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 115068 [details] [review] Bug 26947: Replace double quotes from comments Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 115069 [details] [review] Bug 26947: Correct path for chown and chmod Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 115070 [details] [review] Bug 26947: Update kohastructure.sql Hum, I was going to push it and found: % diff /var/spool/koha/kohadev/kohadev-schema-2021-01-12.sql installer/data/mysql/kohastructure.sql 1c1 < -- MySQL dump 10.16 Distrib 10.1.47-MariaDB, for debian-linux-gnu (x86_64) --- > -- MariaDB dump 10.18 Distrib 10.5.8-MariaDB, for debian-linux-gnu (x86_64) 5c5 < -- Server version 10.1.48-MariaDB-1~bionic --- > -- Server version 10.3.27-MariaDB-1:10.3.27+maria~focal 28,31c28,31 < `can_be_added_manually` tinyint(4) NOT NULL DEFAULT '1', < `credit_number_enabled` tinyint(1) NOT NULL DEFAULT '0' COMMENT 'Is autogeneration of credit number enabled for this credit type', < `is_system` tinyint(1) NOT NULL DEFAULT '0', < `archived` tinyint(1) NOT NULL DEFAULT '0' COMMENT 'boolean flag to denote if this till is archived or not', --- > `can_be_added_manually` tinyint(4) NOT NULL DEFAULT 1, > `credit_number_enabled` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'Is autogeneration of credit number enabled for this credit type', > `is_system` tinyint(1) NOT NULL DEFAULT 0, > `archived` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'boolean flag to denote if this till is archived or not', [SKIP] % diff /var/spool/koha/kohadev/kohadev-schema-2021-01-12.sql installer/data/mysql/kohastructure.sql | wc -l 1970 Default integer values got quoted in this version of MariaDB (10.1.48-MariaDB-1~bionic), but wasn't in newer version (10.3.27-MariaDB-1:10.3.27+maria~focal) Created attachment 115073 [details] [review] Bug 26947: Remove AUTO_INCREMENT Sending a cake there https://bugs.mysql.com/bug.php?id=20786 (In reply to Jonathan Druart from comment #51) > Default integer values got quoted in this version of MariaDB > (10.1.48-MariaDB-1~bionic), but wasn't in newer version > (10.3.27-MariaDB-1:10.3.27+maria~focal) See https://jira.mariadb.org/browse/MDEV-15377 Deciding to push anyway, it will still be easier to compare and is going to be fixed with higher versions. Pushed to master for 21.05, thanks to everybody involved! Oh big changes in a very important file. Is this safe for backport to stable branches ? (In reply to Fridolin Somers from comment #55) > Oh big changes in a very important file. > Is this safe for backport to stable branches ? No, I would not backport it. (In reply to Jonathan Druart from comment #56) > (In reply to Fridolin Somers from comment #55) > > Oh big changes in a very important file. > > Is this safe for backport to stable branches ? > > No, I would not backport it. Ok no backport :) Reading this back while looking at changes between structure and actual tables, I am not so sure whether this patch (Move -- to COMMENT) was such a good idea. Note that this is an understatement. When you add a COMMENT to structure, you should add a db revision too with that clause for that field. Note that DBIx schema files also include those comments. Since this patch set only touched structure and did not add dbrev's, we will have as a result a very large set of differences between DBIx schema and the tables. I have been working on resolving differences for a 20.11 database, but I am now already looking forward to the massive output of the dbix script when I upgrade to 21.05 or higher ;) Note that running dbix script to get those differences would be very useful but will be hugely obscured by the move in this patch set. What can be done about it? Remove the COMMENT clauses again (revert) ? Adjust dbix script to remove comments (feels like a hack) ? There are a few DBIx loader options that could be considered: generate_pod => 0 This seems to be overkill, too rigorous pod_comment_mode => switching between NAME and DESCRIPTION, not what we need table_comments_table and column_comments_table => adding those tables would make DBIx look first there instead of the COMMENT in the data source Ha, stumbled over this report again now. Why does 20.11 not accept my change in kohastructure, and why does master do? commit affa3164eb475ff09654c690c9df810be8dc670f Author: Martin Renvoize <martin.renvoize@ptfs-europe.com> Date: Wed Nov 18 17:08:11 2020 +0000 Bug 26947: (follow-up) Set key checks to disabled for kohastructure + # Disable checks before load + $self->{'dbh'}->do(q{SET NAMES utf8mb4}); + $self->{'dbh'}->do(q{SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0}); + $self->{'dbh'}->do(q{SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0}); + $self->{'dbh'}->do(q{SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO'}); + $self->{'dbh'}->do(q{SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0}); This latter change could have been backported since it actually resolves a problem that existed in Koha since we introduced DBIx::RunSQL. DBIx::RunSQL just silently ignore the first lines in kohastructure.sql that are dependent on version: /*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */; /*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */; /*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */; /*!40101 SET NAMES utf8mb4 */; /*!40103 SET @OLD_TIME_ZONE=@@TIME_ZONE */; /*!40103 SET TIME_ZONE='+00:00' */; /*!40014 SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0 */; /*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */; /*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */; /*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */; I wonder if we should remove them from this sql file now, since they are misleading! They are never executed. (In reply to Marcel de Rooy from comment #61) > This latter change could have been backported since it actually resolves a > problem that existed in Koha since we introduced DBIx::RunSQL. > DBIx::RunSQL just silently ignore the first lines in kohastructure.sql that > are dependent on version: > > /*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */; > /*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */; > /*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */; > /*!40101 SET NAMES utf8mb4 */; > /*!40103 SET @OLD_TIME_ZONE=@@TIME_ZONE */; > /*!40103 SET TIME_ZONE='+00:00' */; > /*!40014 SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0 */; > /*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, > FOREIGN_KEY_CHECKS=0 */; > /*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */; > /*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */; > > I wonder if we should remove them from this sql file now, since they are > misleading! They are never executed. Opened bug 30620 to add at least a comment leaving the possibility to run this directly. And in line with the former comments, the following commit made lots of useless changes in kohastructure: commit 8c5845ef9ecd92584723cd60a221ecd4710a2516 Author: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Date: Fri Dec 4 15:39:32 2020 +0100 Bug 26947: Update kohastructure.sql The following pattern was added: -DROP TABLE IF EXISTS `auth_header`; -CREATE TABLE `auth_header` ( +DROP TABLE IF EXISTS `account_credit_types`; +/*!40101 SET @saved_cs_client = @@character_set_client */; +/*!40101 SET character_set_client = utf8 */; +CREATE TABLE `account_credit_types` ( But note that if we run the file in the usual way via Installer, these lines with /*! pass DBIx::RunSQL and are just IGNORED ! They are useless ! Could you remove them, Jonathan ? (In reply to Marcel de Rooy from comment #63) > And in line with the former comments, the following commit made lots of > useless changes in kohastructure: > > commit 8c5845ef9ecd92584723cd60a221ecd4710a2516 > Author: Jonathan Druart <jonathan.druart@bugs.koha-community.org> > Date: Fri Dec 4 15:39:32 2020 +0100 > > Bug 26947: Update kohastructure.sql > > The following pattern was added: > -DROP TABLE IF EXISTS `auth_header`; > -CREATE TABLE `auth_header` ( > +DROP TABLE IF EXISTS `account_credit_types`; > +/*!40101 SET @saved_cs_client = @@character_set_client */; > +/*!40101 SET character_set_client = utf8 */; > +CREATE TABLE `account_credit_types` ( > > But note that if we run the file in the usual way via Installer, these lines > with /*! pass DBIx::RunSQL and are just IGNORED ! They are useless ! > > Could you remove them, Jonathan ? Ha copied the wrong DROP/CREATE example, but you ll probaly understand it :) (In reply to Marcel de Rooy from comment #63) > And in line with the former comments, the following commit made lots of > useless changes in kohastructure: > Could you remove them, Jonathan ? As said on another bug report, this kohastructure.sql will be auto generated when 22.05.00 will be released. We should not add manual changes to it (apart from the usual changes). If you want to modify it you should adjust mysqldump's output (which was already not trivial if you read the previous comments). Or sed it and modify koha-dump. To still catch differences more easily, I will submitting some code on bug 32334 to sync the comments between actual database and schema. |