Bug 11384 - Koha should not use "ctid" as a column name
Summary: Koha should not use "ctid" as a column name
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 7365
  Show dependency treegraph
 
Reported: 2013-12-11 16:29 UTC by Galen Charlton
Modified: 2015-06-04 23:23 UTC (History)
2 users (show)

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


Attachments
Bug 11384: rename the collections_tracking.ctId column (3.10 KB, patch)
2013-12-11 17:04 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 11384: (follow-up) update DBIC schema classes (2.13 KB, patch)
2013-12-11 17:04 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 11384: rename the collections_tracking.ctId column (3.17 KB, patch)
2013-12-22 22:29 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11384: (follow-up) update DBIC schema classes (2.18 KB, patch)
2013-12-22 22:29 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 11384: rename the collections_tracking.ctId column (3.24 KB, patch)
2013-12-23 10:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11384: (follow-up) update DBIC schema classes (2.24 KB, patch)
2013-12-23 10:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
[3.14.x]Bug 11384: (follow-up) test if table exists in update database (1.22 KB, patch)
2014-02-08 08:54 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Galen Charlton 2013-12-11 16:29:54 UTC
PostgreSQL defines several system columns for each table.  These include oid, tableoid, xmin, cmin, xmax, cmax, and ctid.  These column names cannot be used in a create table statement.

There happens to be one table, collection_tracker, that uses 'ctId' as a column name; this should be renamed, as otherwise DBIC won't be able to add that table to a Pg database.
Comment 1 Galen Charlton 2013-12-11 17:04:04 UTC Comment hidden (obsolete)
Comment 2 Galen Charlton 2013-12-11 17:04:15 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2013-12-22 22:29:42 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2013-12-22 22:29:57 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2013-12-23 10:25:19 UTC
Created attachment 23776 [details] [review]
Bug 11384: rename the collections_tracking.ctId column

'ctId' as a column name conflicts with one of the system
columns that PostgreSQL uses for each table, and consequently
needs to be renamed to enable deploying the schema to a Pg
database.  This patch makes this change.

To test:

[1] Apply the patch and run the SQL specified in the database
    updated.
[2] Verify that the collections_tracking table no longer has
    a ctId column, but now has collections_tracking_id.
[3] Verify that prove -v t/db_dependent/RotatingCollections.t
    passes.
[4] Verify that installer/data/mysql/kohastructure.sql runs
    cleanly in an empty database.

This patch does not affect user-visible behavior given the fact
that the rotating collections feature is currently disabled.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 6 Jonathan Druart 2013-12-23 10:25:27 UTC
Created attachment 23777 [details] [review]
Bug 11384: (follow-up) update DBIC schema classes

This patch updates the DBIC schema class for CollectionTracking
to reflect the new name of its primary key column.

To test:

The CollectionTracking class is not currently used, but
if you *really* want to test this, take a look at the following
branch: http://git.librarypolice.com/?p=koha-galen.git;a=shortlog;h=refs/heads/pg

Then, set up a PostgreSQL database, update koha-conf.xml to point to it,
then run pg/deploy and verify that the collections_tracking table is created
in the Pg database.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 7 Galen Charlton 2013-12-23 16:17:56 UTC
Pushed to master.
Comment 8 Fridolin Somers 2014-01-03 08:30:31 UTC
Patches pushed to 3.14.x, will be in 3.14.2.
(created version 3.14.01.002)
Comment 9 Chris Nighswonger 2014-02-07 17:14:05 UTC
FWIW, upgrading koha-common from 3.14.01 to 3.14.03 threw the following:

Upgrade to 3.14.01.001 done (Bug 10970 - Update MARC21 frameworks to Update Nr. 17 - DB update)
DBD::mysql::db do failed: Table 'koha_biblios.collections_tracking' doesn't exist at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 7830.

It is probably a good idea to trap such errors if they are expected by testing for the existence of the table perior to operating on it.
Comment 10 Fridolin Somers 2014-02-08 08:54:54 UTC
Created attachment 25154 [details] [review]
[3.14.x]Bug 11384: (follow-up) test if table exists in update database

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 11 Fridolin Somers 2014-02-08 08:56:53 UTC
(In reply to Chris Nighswonger from comment #9)
> It is probably a good idea to trap such errors if they are expected by
> testing for the existence of the table perior to operating on it.
I've pushed a patch that tests table existence into 3.14.x branch.
Comment 12 Katrin Fischer 2014-02-09 06:06:21 UTC
While testing for the existance of the table is good - it worries me a bit, that the table is not there. I got the table on my installations.
Comment 13 Fridolin Somers 2014-02-10 07:49:46 UTC
(In reply to Katrin Fischer from comment #12)
> While testing for the existance of the table is good - it worries me a bit,
> that the table is not there. I got the table on my installations.
Indeed, this table exists since 3.2
Comment 14 Chris Nighswonger 2014-02-10 19:15:31 UTC
(In reply to Fridolin SOMERS from comment #13)
> (In reply to Katrin Fischer from comment #12)
> > While testing for the existance of the table is good - it worries me a bit,
> > that the table is not there. I got the table on my installations.
> Indeed, this table exists since 3.2

Interesting and a bit disconcerning since this DB began with a 3.0 installation and has moved along ever since. It provokes a few not-necessarily-new thoughts:

1. We should probably "stamp" a new database when it is created with the original rev number it was created under. This field would never change and could be used to somehow "verify" that the current DB structure at any point in time was, indeed, current.

2. There should definitely be some mechanism in place to veryify the existence of tables prior to "working" on them during upgrades.

3. When dealing with essential tables, there should be a mechanism to throw a rather large warning to the user when one of them is found missing, even if there is no "easy" way to fix it because what you don't know can, indeed, hurt you.

But we have beat around this bush before, so I just list them here for posterity's sake. :-)