Bug 12558 - DBIx::Class schema update 07/2014
Summary: DBIx::Class schema update 07/2014
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-11 13:10 UTC by Tomás Cohen Arazi
Modified: 2016-06-21 21:37 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12558: DBIx::Class schema update 07/2014 (30.64 KB, patch)
2014-07-11 13:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12558: Require DBIx::Schema::Loader v0.07039 or higher (917 bytes, patch)
2014-07-11 13:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12558: DBIx::Class schema update 07/2014 (30.69 KB, patch)
2014-07-11 14:50 UTC, Yohann Dufour
Details | Diff | Splinter Review
Bug 12558: Require DBIx::Schema::Loader v0.07039 or higher (972 bytes, patch)
2014-07-11 14:50 UTC, Yohann Dufour
Details | Diff | Splinter Review
[PASSED QA] Bug 12558: DBIx::Class schema update 07/2014 (30.79 KB, patch)
2014-07-11 15:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 12558: Require DBIx::Schema::Loader v0.07039 or higher (1.02 KB, patch)
2014-07-11 15:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12558: DBIx::Class schema update 07/2014 (30.84 KB, patch)
2014-07-11 19:24 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 12558: Require DBIx::Schema::Loader v0.07039 or higher (1.13 KB, patch)
2014-07-11 19:24 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 12558: (follow-up) remove old versions of schema classes for two tables (4.09 KB, patch)
2014-07-11 19:24 UTC, Galen Charlton
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2014-07-11 13:10:47 UTC
As Yohann stated, there have been several changes in the DB structure since last update of the Koha::Schema::Result::* classes.

Recent DBIC::Schema::Loader (v0.07039) generates better code as Galen expressed in his post to koha-devel:

"When I was RM for 3.16, I was using DBIC::Schema::Loader version
v0.07025.  Yesterday Tomas and I did some checking of classes
generated using v0.07039.  Among other changes, the newer versions now
recognize "set null" as a FK action and default to "restrict" rather
than "cascade" if the action is not explicitly specified."

"Those are both good changes, so I recommend that we require at least
version v0.07039 of DBIC::Schema::Loader.  Since that module is /not/
required for production use -- it's only needed for development --
requiring a hire version should not affect packaging signficantly."

So we are:
- Updating the DBIx::Class schema
- Shifting DBIC::Schema::Loader version requirement to v0.07039
Comment 1 Tomás Cohen Arazi 2014-07-11 13:16:25 UTC
DBIC::Schema::Loader version requirement to v0.07039
Comment 2 Tomás Cohen Arazi 2014-07-11 13:18:44 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2014-07-11 13:18:51 UTC Comment hidden (obsolete)
Comment 4 Yohann Dufour 2014-07-11 14:50:16 UTC Comment hidden (obsolete)
Comment 5 Yohann Dufour 2014-07-11 14:50:25 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2014-07-11 15:47:30 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2014-07-11 15:47:55 UTC Comment hidden (obsolete)
Comment 8 Galen Charlton 2014-07-11 19:24:26 UTC
Created attachment 29655 [details] [review]
Bug 12558: DBIx::Class schema update 07/2014

Update DBIx::Class schema classes to use a newer
version of DBIx::Class:Schema::Loader.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Yohann Dufour <dufour.yohann@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Comment 9 Galen Charlton 2014-07-11 19:24:31 UTC
Created attachment 29656 [details] [review]
Bug 12558: Require DBIx::Schema::Loader v0.07039 or higher

This patch shifts required DBIx::Schema::Loader to a newer
version to take advantage of improved smarts about foreign
key relationships.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Yohann Dufour <dufour.yohann@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Comment 10 Galen Charlton 2014-07-11 19:24:36 UTC
Created attachment 29657 [details] [review]
Bug 12558: (follow-up) remove old versions of schema classes for two tables

The newer version of DBIx::Class::Schema::Loader tweaks
how it maps plural table names to class names.  As a consequence,
two (currently-unused) classes are renamed.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Comment 11 Tomás Cohen Arazi 2014-07-11 19:31:10 UTC
Patches pushed to master.
Comment 12 David Cook 2014-11-27 23:42:09 UTC
I'm noticing a fair number of issues with the use of DBIx::Class::Schema::Loader over at http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11390.

A summary of the most recent issues I've found:

1) "ON UPDATE CURRENT_TIMESTAMP" doesn't make it in to the DBIC classes.
2) "All int(egers) are recorded as int(11), all tinyint as tinyint(4), all smallint as smallint(6).
3) Many indexes are missing (which causes `items_search_fields` and `message_queue` not to create during a deploy because of foreign_key and auto_increment problems respectively)
4) Extra indexes appear (as all foreign keys are indexed by default, which isn't something we've done consistently in kohastructure.sql, although it's a good idea)
5) Index names and foreign key constraint names are different between DBIC and kohastructure.sql
6) DBIC doesn't include COMMENTs (honestly, I don't think this matters at all)
7) (UTF8) CHARACTER SET and COLLATE do not appear in the SQL deployed by DBIC
--
8) We've added extra foreign keys to the DBIC classes but not to kohastructure.sql... 
9) We've added the `closure` and `closure_rrule` tables to DBIC but not to kohastructure.sql
--
10) `pending_offline_operations` is loaded as an InnoDB table but is actually a MyISAM table...
11) The 'double' data type is recorded as 'double precision' which causes massive problems when using SQL::Translator::Diff (which is used by DBIx::Class::Schema::create_ddl_dir() to make SQL diffs between schema versions)

-----

I see DBIx::Class::Schema::Loader being responsible for:

1) "ON UPDATE CURRENT_TIMESTAMP" doesn't make it in to the DBIC classes (I think this can be defined as an "extra" in the Result classes themselves... requires more investigation)
2) "All int(egers) are recorded as int(11), all tinyint as tinyint(4), all smallint as smallint(6). 
-> Actually, the "int" data types have the number stripped off by DBIC::Schema::Loader. The problem with the number width/range happens during DBIC::Schema->deploy()
-> The numbering happens thanks to SQL::Translator::Producer::MySQL I think...requires more investigation... I think this might be solved by adding "size" to the DBIC Result classes manually... or possibly via the Loader...
3) Missing indexes (requires investigation)
5) Index names and foreign key constraint names are different... (currently investigating this one)
6) DBIC doesn't include COMMENTs (I don't know how it would or if it could. But again, I don't think this matters.)
7) (UTF8) CHARACTER SET and COLLATE do not appear in the SQL deployed by DBIC (I don't really know anything about character sets or collation, so... hoping someone can help out on this one)
10) `pending_offline_operations` is loaded as an InnoDB table but is actually a MyISAM table... (I think there might be filtering available for this... investigating this too)
11) The 'double' data type is recorded as 'double precision' which causes massive problems when using SQL::Translator::Diff (which is used by DBIx::Class::Schema::create_ddl_dir() to make SQL diffs between schema versions)