See attached notifications.install patch file for preliminary PostgreSQL support.

I have note tested the module itself to check for query failures, just tested the table creation statements. Consider this a first step for more extensive testing of PostgreSQL support.

Comments

jose reyero’s picture

Status: Needs review » Fixed

Cool, thank you.

Now the module installs for pgsql I hope we'll get more feedback about that :-)

We are not using Postgres, so I'll have rely on other people's feedback for pgsql related patches.

jaydub’s picture

Status: Fixed » Needs review
StatusFileSize
new711 bytes

I patched notfications.cron.inc in the notifications_process_queue() function. PostgreSQL is a bit of stickler about GROUP BY/ORDER BY and requires in this case the ORDER BY column to also appear in the GROUP BY clause.

From what I can tell this shouldn't affect the query results but you would know better than I...

jose reyero’s picture

Status: Needs review » Fixed

Not that I understand the pgsql rationale for that, but this doesn't seem to break anything, so I've applied your patch.
Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

jaydub’s picture

Status: Closed (fixed) » Needs review
StatusFileSize
new781 bytes

Re-opening this issue as there is another case where a GROUP BY clause in a query needs to
include all columns in the SELECT clause for PostgreSQL.

See attached patch to current -dev

jose reyero’s picture

Status: Needs review » Fixed

Thanks, committed

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.