We are moving to Git Issues for bug tracking in future releases. During transition, content will be in both tools. If you'd like to file a new bug, please create an issue.

Bug 6097

Summary: Unexpected counter log data loss on upgrade
Product: OJS Reporter: Colin Prince <colin.prince>
Component: InstallerAssignee: PKP Support <pkp-support>
Status: RESOLVED WORKSFORME    
Severity: normal CC: a.marchitelli, alec, jerico.dev, marini
Priority: P5    
Version: 2.4.x   
Hardware: All   
OS: All   
Version Reported In: Also Affects:
Attachments: Patch against OJS 2.3.3

Description Colin Prince 2010-11-01 13:01:05 PDT
During upgrade (when using Postgres) from 2.2.4 to 2.3.3 the upgrade executes unexpected SQL:

DROP INDEX counter_monthly_log_pkey
ALTER TABLE counter_monthly_log DROP COLUMN "count_jan"
ALTER TABLE counter_monthly_log DROP COLUMN "count_feb"
ALTER TABLE counter_monthly_log DROP COLUMN "count_mar"
ALTER TABLE counter_monthly_log DROP COLUMN "count_apr"
ALTER TABLE counter_monthly_log DROP COLUMN "count_may"
ALTER TABLE counter_monthly_log DROP COLUMN "count_jun"
ALTER TABLE counter_monthly_log DROP COLUMN "count_jul"
ALTER TABLE counter_monthly_log DROP COLUMN "count_aug"
ALTER TABLE counter_monthly_log DROP COLUMN "count_sep"
ALTER TABLE counter_monthly_log DROP COLUMN "count_oct"
ALTER TABLE counter_monthly_log DROP COLUMN "count_nov"
ALTER TABLE counter_monthly_log DROP COLUMN "count_dec"
ALTER TABLE counter_monthly_log DROP COLUMN "count_ytd_total"
ALTER TABLE counter_monthly_log DROP COLUMN "count_ytd_html"
ALTER TABLE counter_monthly_log DROP COLUMN "count_ytd_pdf"
BEGIN
SELECT * INTO TEMPORARY TABLE counter_monthly_log_tmp FROM counter_monthly_log
DROP TABLE counter_monthly_log CASCADE
CREATE TABLE counter_monthly_log (
year                     INT8 NOT NULL,
journal_id               INT8 NOT NULL
)
INSERT INTO counter_monthly_log (year, journal_id) SELECT year, journal_id FROM counter_monthly_log_tmp
DROP TABLE counter_monthly_log_tmp
COMMIT
ALTER TABLE counter_monthly_log ADD COLUMN month INT8 
ALTER TABLE counter_monthly_log ALTER COLUMN month SET NOT NULL
ALTER TABLE counter_monthly_log ADD COLUMN count_html INT8  
UPDATE counter_monthly_log SET count_html=0
ALTER TABLE counter_monthly_log ALTER COLUMN count_html SET DEFAULT 0
UPDATE counter_monthly_log SET count_html = '0' WHERE count_html IS NULL
ALTER TABLE counter_monthly_log ALTER COLUMN count_html SET NOT NULL
ALTER TABLE counter_monthly_log ADD COLUMN count_pdf INT8  
UPDATE counter_monthly_log SET count_pdf=0
ALTER TABLE counter_monthly_log ALTER COLUMN count_pdf SET DEFAULT 0
UPDATE counter_monthly_log SET count_pdf = '0' WHERE count_pdf IS NULL
ALTER TABLE counter_monthly_log ALTER COLUMN count_pdf SET NOT NULL
ALTER TABLE counter_monthly_log ADD COLUMN count_other INT8  
UPDATE counter_monthly_log SET count_other=0
ALTER TABLE counter_monthly_log ALTER COLUMN count_other SET DEFAULT 0
UPDATE counter_monthly_log SET count_other = '0' WHERE count_other IS NULL
ALTER TABLE counter_monthly_log ALTER COLUMN count_other SET NOT NULL
CREATE UNIQUE INDEX counter_monthly_log_key ON counter_monthly_log (year, month, journal_id)

reported in the forum:

http://pkp.sfu.ca/support/forum/viewtopic.php?f=8&t=6756
Comment 1 Colin Prince 2010-11-01 13:08:27 PDT
For reference:

http://github.com/pkp/ojs/blob/ojs-2_3_3-3/plugins/generic/counter/counter_monthly_log_1_1.xml

There's no postgres specific code yet, only mysql.
Comment 2 Alec Smecher 2010-11-01 18:49:00 PDT
Colin, those ALTER TABLE statements are executed when the database descriptor is synced against the database (see dbscripts/xml/upgrade.xml); if you need to change the way the counter logs are stored, that step in the upgrade will need to be executed before the descriptor is synced or else the columns will be dropped and the data lost.
Comment 3 Colin Prince 2010-11-02 11:12:31 PDT
Created attachment 3319 [details]
Patch against OJS 2.3.3

Adding the counter_monthly_log_1_1.xml to dbscripts/xml/upgrade.xml and adding support for Postgres
Comment 4 Colin Prince 2010-11-03 18:21:35 PDT
Committed to official:

https://github.com/pkp/ojs/commit/a68cf42c204479bc0ee13e2609d2d5e88ffcc0e5
Comment 5 jerico 2010-11-05 09:27:29 PDT
Colin, I don't think that your solution is idempotent (=can be executed several times without error always leading to the correct end result). Idempotency is important in case something goes wrong at in the update process and users have to repeat it. Users should be able to re-execute the update process without error even if it has been partially executed before.

Why don't you move your preparatory changes to a preupdate file instead so that you can use the normal schema synchronization mechanism? There are several preupdate files in OJS that you can use as samples. I'll forward an email about idempotency (if I find it) with more information.
Comment 6 jerico 2010-11-10 06:52:38 PST
Non-idempotency has been confirmed. Reopened until implemented.
Comment 7 Alec Smecher 2010-12-09 12:33:37 PST
Colin, I spotted some space-based indents in your fix here -- make sure your editor is using tabs.

Also, I've encountered OJS 2.3.0 installs that don't have a counter_monthly_log table. I'd suggest making this upgrade conditional on that table existing. (See the "condition" attribute in dbscripts/xml/upgrade.xml for that purpose.) Also, the CREATE TABLE statement shouldn't be hard-coded in XML -- use a schema.xml file instead.
Comment 8 Andrea Marchitelli 2011-01-06 13:43:00 PST
Hi, 
our user are asking for COUNTER plugin, that we can not enable since there are some issues opened about it.
Do you have any news about the idempotency problem solution?

We need absolutely to enable COUNTER statistics in a few days.

Thank you for helping us.

Andrea
Comment 9 Andrea Marchitelli 2011-01-31 09:20:28 PST
Hi,
we succesfully used this method with Postgres (thank to Colin):
1. Copy from the old DB the table counter_monthly_log into the new as  counter_monthly_log_backup
2. drop the counter_monthly_log table and recreate it:
CREATE TABLE counter_monthly_log
(
  "year" bigint,
  "month" bigint,
  journal_id bigint,
  count_html bigint,
  count_pdf bigint,
  count_other bigint,
  CONSTRAINT counter_monthly_log_01 PRIMARY KEY ("year", "month", journal_id)
)
WITHOUT OIDS;

3. Then we would do a SELECT from the old table and put it in the new table.
INSERT INTO counter_monthly_log
(year,month,journal_id,count_html,count_pdf,count_other)
SELECT year, 1 as month, journal_id, 0, count_jan as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 2 as month, journal_id, 0, count_feb as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 3 as month, journal_id, 0, count_mar as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 4 as month, journal_id, 0, count_apr as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 5 as month, journal_id, 0, count_may as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 6 as month, journal_id, 0, count_jun as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 7 as month, journal_id, 0, count_jul as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 8 as month, journal_id, 0, count_aug as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 9 as month, journal_id, 0, count_sep as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 10 as month, journal_id, 0, count_oct as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 11 as month, journal_id, 0, count_nov as count, 0 FROM
counter_monthly_log_backup
UNION
SELECT year, 12 as month, journal_id, 0, count_dec as count, 0 FROM
counter_monthly_log_backup
ORDER BY journal_id, year, month
Comment 10 Colin Prince 2011-02-03 12:04:10 PST
As Andrea just pointed out, for this to work properly, the count* fields should be not null and default to 0.

So step 2 in the manual process should be:

CREATE TABLE counter_monthly_log
(
  "year" bigint,
  "month" bigint,
  journal_id bigint,
  count_html bigint NOT NULL DEFAULT 0,
  count_pdf bigint NOT NULL DEFAULT 0,
  count_other bigint NOT NULL DEFAULT 0,
  CONSTRAINT counter_monthly_log_01 PRIMARY KEY ("year", "month", journal_id)
)
WITHOUT OIDS;

This is already specified in the schema so no change required there.
Comment 11 Alec Smecher 2014-06-25 08:37:26 PDT
Deprecated by stats rewrite.