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 8769 - Bad SQL for PostgreSQL in Timed View stats migration
Bad SQL for PostgreSQL in Timed View stats migration
Status: NEW
Product: OJS
Classification: Unclassified
Component: Installer
3.x
All All
: P3 normal
Assigned To: beghelli
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-05-21 05:57 PDT by Tom Christensen
Modified: 2014-08-14 16:12 PDT (History)
1 user (show)

See Also:
Version Reported In:
Also Affects:


Attachments
Patch for OJS 2.4.4-1 (2.20 KB, patch)
2014-05-21 05:58 PDT, Tom Christensen
Details | Diff
Alternative fix for OJS 2.4.4-1 (3.06 KB, patch)
2014-08-07 16:31 PDT, Alec Smecher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Christensen 2014-05-21 05:57:48 PDT
Upgrading from 2.4.2 to 2.4.4-1 I see this error:
ojs2 has produced an error
  Message: WARNING: pg_query_params(): Query failed: ERROR:  column "a.journal_id" must appear in the GROUP BY clause or be used in an aggregate function at character 347
  In file: /home/ojs/stage-current/lib/pkp/lib/adodb/drivers/adodb-postgres7.inc.php
  At line: 124
  Stacktrace: 
  Server info:
   OS: Linux
   PHP Version: 5.1.6
   Apache Version: N/A
   DB Driver: postgres
   DB server version: PostgreSQL 8.4.20 on i386-redhat-linux-gnu, compiled by GCC gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-54), 32-bit
-1: ERROR:  column "a.journal_id" must appear in the GROUP BY clause or be used in an aggregate function at character 347
                                                                ADOConnection._Execute(INSERT INTO metrics (load_id, metric_type, assoc_type, assoc_id, day, month, country_id, region, city, submission_id, metric, co..., Array[3])% line  860, file: /home/ojs/stage-current/lib/pkp/lib/adodb/adodb.inc.php
                                                        ADOConnection.Execute(INSERT INTO metrics (load_id, metric_type, assoc_type, assoc_id, day, month, country_id, region, city, submission_id, metric, co..., Array[3])% line  226, file: /home/ojs/stage-current/lib/pkp/classes/db/DAO.inc.php
                                                DAO.update(INSERT INTO metrics (load_id, metric_type, assoc_type, assoc_id, day, month, country_id, region, city, submission_id, metric, co..., Array[3])% line 1149, file: /home/ojs/stage-current/classes/install/Upgrade.inc.php
                                        Upgrade.migrateTimedViewsUsageStatistics(Object:Upgrade, Array[2])% line    0, file: 
                                call_user_func(Array[2], Object:Upgrade, Array[2])% line  419, file: /home/ojs/stage-current/lib/pkp/classes/install/Installer.inc.php
<h1>DB Error: ERROR:  column "a.journal_id" must appear in the GROUP BY clause or be used in an aggregate function at character 347</h1>ojs2: DB Error: ERROR:  column "a.journal_id" must appear in the GROUP BY clause or be used in an aggregate function at character 347

I can only guess that PostgreSQL is more strict here than MySQL.
Also working around this issue it will then complain about pa_issue.id from the same SELECT.
The galley migration following runs into the same issue.

A quick fix that allows me to go further is wrapping the affected fields in an aggregate function such as max() but I'm not sure if it is the right fix.
I'll attach a patch that does this.

For reference on how PostgreSQL treats GROUP BY:
http://www.postgresql.org/docs/8.4/static/queries-table-expressions.html
Comment 1 Tom Christensen 2014-05-21 05:58:17 PDT
Created attachment 4023 [details]
Patch for OJS 2.4.4-1
Comment 2 Alec Smecher 2014-08-06 16:44:10 PDT
Thanks for contributing, Tom, and sorry for the delay.

It looks like...

- The LEFT JOINs for articles (both queries), published_articles (both queries), and and article_galleys (last query) can be converted to normal JOINs, meaning that we can remove the WHERE conditions:
  AND a.journal_id IS NOT NULL AND pa.issue_id IS NOT NULL (both queries)

- We can add the following to GROUP BY:
  First query: a.journal_id, pa.issue_id
  Second query: ag.article_id, a.journal_id, pa.issue_id

- We can therefore leave the columns above specified in the SELECT part of the clause, since they're grouped, without needing to use aggregate functions.

I'll upload a patch that does this shortly. Tom, is there any chance you could run a test upgrade with the new patch applied? I don't have a sample PostgreSQL dataset that covers the timed views plugin.
Comment 3 Alec Smecher 2014-08-07 16:31:18 PDT
Created attachment 4049 [details]
Alternative fix for OJS 2.4.4-1

Bruno, could you review this patch?
Comment 4 Alec Smecher 2014-08-07 16:52:47 PDT
For my own reference, a useful command line to convert mysql data for ingest into psql -- data only, no schema:

(echo "SET standard_conforming_strings = 'off'; SET backslash_quote = 'on';"; mysqldump --skip-lock-tables -c -t -u ojs2 -p --compatible=ansi ojs2upgrade) | iconv -f utf8 -t utf8//IGNORE | psql -U ojs2 ojs2upgrade
Comment 5 Tom Christensen 2014-08-13 04:00:45 PDT
(In reply to Alec Smecher from comment #2)
> I'll upload a patch that does this shortly. Tom, is there any chance you
> could run a test upgrade with the new patch applied? I don't have a sample
> PostgreSQL dataset that covers the timed views plugin.

I've just run an upgrade with the new patch installed ontop of the existing patch set for 2.4.4-1:
85d2ac3 Bug #8768: fix date conversion for PostgresSQL
44e6bbf Bug #8770: Properly UTF-8 encode GeoIP data
6174709 Bug #8771: Suppress out-of-bound array index warning
bc501a0 Bug #8764: Fix method usage
7c7610c Bug #8763: fix Postgres SQL for timed views upgrade
d65e84e Merge branch 'upstream_2.4.x' into maint_2.4.x
88b458f Import OJS 2.4.4-1

I used a fresh dump from our 2.4.2 production instance and verified that it would fail without the patch. After applying the patch the upgrade completed without errors.

Thank you for following up on this.
Comment 6 Alec Smecher 2014-08-14 16:09:02 PDT
Pull request opened (not merged):
Introducing patch create by Alec
https://github.com/pkp/ojs/pull/263
Comment 7 beghelli 2014-08-14 16:09:02 PDT
Introducing patch create by Alec
https://github.com/pkp/ojs/commit/64aecb20272731bf3c715a2e52c239b6812f87a8
Comment 8 Alec Smecher 2014-08-14 16:09:02 PDT
Pull request closed (merged):
Introducing patch create by Alec
https://github.com/pkp/ojs/pull/263
Comment 9 beghelli 2014-08-14 16:12:03 PDT
Need porting to master