Another PostgreSQL bug: series links

[crism]

From https://iopn.library.illinois.edu/books/pww/ follow either series link, e.g. https://iopn.library.illinois.edu/books/pww/catalog/series/afropww, see SQL error. This is the same kind of SQL pedantry as all the other PostgreSQL ones…

[asmecher]

@crism, could you turn on the stack trace option in config.inc.php and post the trace here (removing anything you don't want preserved)? Once the error message goes away this issue entry will be pretty hard to decypher. Thanks!

[crism]

From our test installation error log:

[Wed Feb 27 00:34:37.527204 2019] [:error] omp: DB Error: ERROR:  for SELECT DISTINCT, ORDER BY expressions must appear in select list
LINE 1: ...f.seq is null then 1 else 0 end, "psf"."seq" ASC, "st"."sett...
                                                             ^, referer: .../index.php/sandbox/index

From the stack trace:

Stack Trace:
File: .../omp/lib/pkp/classes/services/PKPSubmissionService.inc.php line 67
Function: DAO->retrieveRange("select distinct "s".*, "ps".*, COALESCE(stl.setting_value, stpl....", Array(4), Object(DBResultRange))

File: .../omp/pages/catalog/CatalogHandler.inc.php line 260
Function: PKP\Services\PKPSubmissionService->getSubmissions("1", Array(8))

File: (unknown) line (unknown)
Function: CatalogHandler->series(Array(1), Object(Request))

File: .../omp/lib/pkp/classes/core/PKPRouter.inc.php line 390
Function: call_user_func(Array(2), Array(1), Object(Request))

File: .../omp/lib/pkp/classes/core/PKPPageRouter.inc.php line 232
Function: PKPRouter->_authorizeInitializeAndCallRequest(Array(2), Object(Request), Array(1), False)

File: .../omp/lib/pkp/classes/core/Dispatcher.inc.php line 134
Function: PKPPageRouter->route(Object(Request))

File: .../omp/lib/pkp/classes/core/PKPApplication.inc.php line 247
Function: Dispatcher->dispatch(Object(Request))

File: .../omp/index.php line 64
Function: PKPApplication->execute()

[crism]

The full query:

select
  distinct "s".*,
  "ps".*,
  COALESCE(stl.setting_value, stpl.setting_value) AS series_title,
  "psf"."seq",
  case when psf.seq is null then 1 else 0 end
from
  "submissions" as "s"
  left join "submission_settings" as "st" on "s"."submission_id" = "st"."submission_id"
  left join "published_submissions" as "ps" on "ps"."submission_id" = "s"."submission_id"
  left join "series_settings" as "stpl"
    on "s"."series_id" = stpl.series_id
    and "stpl"."setting_name" = 'title'
    and "stpl"."locale" = 'en_US'
  left join "series_settings" as "stl"
    on "s"."series_id" = stl.series_id
    and "stl"."setting_name" = 'title'
    and "stl"."locale" = 'en_US'
  left join "features" as "psf"
    on "s"."submission_id" = "psf"."submission_id"
    and "psf"."assoc_type" = 530
    and "psf"."assoc_id" = 1
where
  "s"."context_id" = ?
  and "st"."setting_name" = ?
  and "ps"."pub_id" is not null
  and "s"."status" in (?)
  and "s"."series_id" in (?)
group by
  "s"."submission_id",
  "st"."setting_value",
  "ps"."date_published",
  "ps"."pub_id",
  COALESCE(stl.setting_value, stpl.setting_value),
  psf.seq
order by
  case when psf.seq is null then 1 else 0 end,
  "psf"."seq" ASC,
  "st"."setting_value" asc

[asmecher]

What release/branch are you working with?

[crism]

We’re on 3.1.1-4 (though we have two patches installed: fc10972 (pkp/omp) and pkp/omp#601).

[crism]

Adding "st"."setting_value" to the select clause of the above query fixes the error in interactive PostgreSQL. However, the CatalogHandler and PKPSubmissionService have changed a fair bit since 3.1.1-4, and I’m having trouble untangling where exactly to make that change.

[crism]

This is a specific example of the more general problem, though, that we keep throwing things into GROUP BY and ORDER BY clauses that aren’t in SELECT, which violates SQL rules that PostgreSQL actually enforces. We ought to be more careful.

[asmecher]

Hmm, I wonder if this is already solved by #4411...

[asmecher]

We ought to be more careful.

We'd definitely catch these more quickly if part of the dev team used PostgreSQL on the regular. (I have it set up for test scripting, but that's it). The best solution will be to improve Travis test coverage, which runs PostgreSQL alongside MySQL.

[NateWr]

Closing this as outdated. If you can reproduce this on a recent stable version, please provide reproduction steps and we can re-open this.