Bug 7365 - Koha should support PostgreSQL
Summary: Koha should support PostgreSQL
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 7802 11390 11422 7785 7806 8164 11155 11384 11389 11391
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-18 09:30 UTC by Marc Balmer
Modified: 2020-06-17 02:31 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Use 'pgsql' as synonym for 'Pg', remove some trailing whitespace (10.46 KB, patch)
2011-12-18 09:39 UTC, Marc Balmer
Details | Diff | Splinter Review
A list of mysqlisms in C4 (2.36 KB, text/plain)
2011-12-18 09:50 UTC, Marc Balmer
Details
Add DBQ, a small module to generate DB specific SQL code (12.30 KB, patch)
2012-03-21 15:06 UTC, Stéphane Delaune
Details | Diff | Splinter Review
Signed off patch to add DBQ (12.34 KB, patch)
2012-03-21 15:42 UTC, Marc Balmer
Details | Diff | Splinter Review
proposed patch (updated) (17.06 KB, patch)
2012-03-22 16:30 UTC, Stéphane Delaune
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Balmer 2011-12-18 09:30:27 UTC
This is a "meta" bug to track ongoing activity and changes needed to make Koha run on a PostgreSQL database.
Comment 1 Marc Balmer 2011-12-18 09:39:31 UTC Comment hidden (obsolete)
Comment 2 Marc Balmer 2011-12-18 09:44:49 UTC
I created a Wiki page to comment on the ongoing work to make Koha run happily with PostgreSQL: http://wiki.koha-community.org/wiki/PostgreSQL
Comment 3 Marc Balmer 2011-12-18 09:50:04 UTC
Created attachment 6852 [details]
A list of mysqlisms in C4

An (incomplete) list of MySQLisms in C4.
Comment 4 Jared Camins-Esakov 2012-02-12 21:47:22 UTC
You have a perplexing change in the patch:
+        $isbn = $isbn->as_isbn10 if $isbn->tHOW COLUMNS FROMype eq 'ISBN13';

Perhaps a typo? Please submit a revised patch, obsolete the existing patch, and change the status to "Needs signoff."
Comment 5 Stéphane Delaune 2012-03-21 15:06:57 UTC Comment hidden (obsolete)
Comment 6 Marc Balmer 2012-03-21 15:42:04 UTC Comment hidden (obsolete)
Comment 7 Jared Camins-Esakov 2012-03-21 19:44:17 UTC
(In reply to comment #5)
> Created attachment 8428 [details] [review]
> Add DBQ, a small module to generate DB specific SQL code
> 
> To eliminate database specific SQL idoms (aka MySQLisms) it is necessary to
> either use a standard SQL form like 'CAST(now() as date)' instead of the
> non-standard MySQLism 'CURDATE()', or to generate a database specific idiom
> where no such standard form exists (e.g. to get the list of column names
> from a table).
> 
> The new DBQ module is a generator for DB specific SQL idioms and comes with
> an implementatio for both MySQL and PostgreSQL.  Generating the SQL idioms
> this way is the least invasive form of getting DB independence, as no large
> changes to the database layer are required.  Please note that DBQ->dbq
> returns a singleton.
> 
> DBQ stands for Database Query.  Name, usage patterns, and implementation
> discussed and agreed upon with many during the Marseille Hackfest 2012.

Why don't we just use one of the existing database abstraction layers available for Perl? Then someone could use a completely different database, if they so desired.
Comment 8 MJ Ray (software.coop) 2012-03-22 09:11:54 UTC
(In reply to comment #7)
> Why don't we just use one of the existing database abstraction layers
> available for Perl? Then someone could use a completely different database,
> if they so desired.

I share this concern. I did a lot of work on the SQL ages ago and still feel that much of it should be replaced by a database-agnostic abstraction layer (if only I had the funding to do it). I don't feel that creating a Koha-specific multi-database SQL generator is a sustainable approach.
Comment 9 Marc Balmer 2012-03-22 09:14:39 UTC
Wwe are aiming at a minimally invasive solution that does not
require us to make obstrusive changes.  And this solution does not mean
to go back to the minimal common available SQL subset, but still be able
to use the full power of the underlying DB.  And it does not prevent us
from changing to a whole other approach in the future.  For now, this is
the bare minimum that is neede to enable Koha to run on different DBs.

Switching do a different DB layer right now does help, because even if we did, Koha still expects MySQL result sets, C4::DBQ takes care of this.
Comment 10 Marc Balmer 2012-03-22 09:19:43 UTC
Signed off the diff.
Comment 11 MJ Ray (software.coop) 2012-03-22 11:28:50 UTC
(In reply to comment #10)
> Signed off the diff.

Huh? Are we allowed to sign off our own enhancements now?

Also, has this approach been discussed on koha-devel?
I remember discussion of DBIx::Class in the past and the related RFC http://wiki.koha-community.org/wiki/RFC_for_using_DBIx_Class_in_Koha but I didn't find similar discussion of C4::DBQ.
Comment 12 Stéphane Delaune 2012-03-22 13:01:46 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > Signed off the diff.
> 
> Huh? Are we allowed to sign off our own enhancements now?
no, but the patch was made by me
Comment 13 Ian Walls 2012-03-22 13:11:12 UTC
Okay, so this patch uses a bolt-on approach.  Three new modules are added (along with a unit test!), along with a new Perl dependency, and then C4/ImportExportFramework.pm is modified to use those modules instead of direct MySQL "SHOW COLUMNS FROM".

It seems like it'd be easier to use $dbh->column_info(undef, undef, $table, '%') or some modification thereof to get this information.  We'd then be using a standardized, packaged database abstraction layer, instead of having to make our own, and it'd give us more than just MySQL and Postgres.  The resulting change would likely be fewer lines of code than what we've got here currently.

Moving to In Discussion, as we've likely still got a lot to talk about in terms of an implementation plan for adding database agnosticity.
Comment 14 Marc Balmer 2012-03-22 13:25:12 UTC
SHOW COLUMNS FROM is only one example of a MySQLism, that we merely used to demonstrate the technique being used.  What we - and we do it on purpose - is to not change the way how the database is accessed, but the SQL being used.  For SHOW COLUMNS FROM there is a function in DBH, but did you check what it returns what the surrounding code in Koha expects?

Well, we did check that and our goals was to minimally invasive, that is why we only change the way the SQL is generated.

How will you address idoms for which no DBH equivalent, no standard SQL equivalent exists, i.e. where different SQL HAS to be used for MySQL or PostgreSQL?  Our solution adresses exactly this problem.  It is small, overseeable, and works.
Comment 15 Stéphane Delaune 2012-03-22 16:30:32 UTC
Created attachment 8534 [details] [review]
proposed patch (updated)

as suggested by Ian Walls the patch replace "SHOW COLMUNS" in ImportExportFramework.pm with $dbh->column_info(undef, undef, $table, "%");
now DBQ implement 2 subs : 
- dateSub (Subtract a time value (interval) from a date)
- ifNull (Returns $a if not null, else return $b)

they are use in sub GetLateOrders in C4/Acquisition.pm .

If db_scheme is mysql, the $query created by trhis sub is :
    SELECT aqbasket.basketno,
        aqorders.ordernumber,
        DATE(aqbasket.closedate)  AS orderdate,
        aqorders.rrp              AS unitpricesupplier,
        aqorders.ecost            AS unitpricelib,
        aqorders.claims_count     AS claims_count,
        aqorders.claimed_date     AS claimed_date,
        aqbudgets.budget_name     AS budget,
        borrowers.branchcode      AS branch,
        aqbooksellers.name        AS supplier,
        aqbooksellers.id          AS supplierid,
        biblio.author, biblio.title,
        biblioitems.publishercode AS publisher,
        biblioitems.publicationyear,
    
    aqorders.quantity - IFNULL(aqorders.quantityreceived, 0) AS quantity,
    (aqorders.quantity - IFNULL(aqorders.quantityreceived, 0)) * aqorders.rrp AS subtotal,
    (CAST(now() AS date) - closedate) AS latesince
     
    FROM
        aqorders LEFT JOIN biblio     ON biblio.biblionumber         = aqorders.biblionumber
        LEFT JOIN biblioitems         ON biblioitems.biblionumber    = biblio.biblionumber
        LEFT JOIN aqbudgets           ON aqorders.budget_id          = aqbudgets.budget_id,
        aqbasket LEFT JOIN borrowers  ON aqbasket.authorisedby       = borrowers.borrowernumber
        LEFT JOIN aqbooksellers       ON aqbasket.booksellerid       = aqbooksellers.id
        WHERE aqorders.basketno = aqbasket.basketno
        AND ( datereceived = ''
            OR datereceived IS NULL
            OR aqorders.quantityreceived < aqorders.quantity
        )
        AND (aqorders.datecancellationprinted IS NULL OR aqorders.datecancellationprinted='0000-00-00')
     AND (closedate <= DATE_SUB(CAST(now() AS date), INTERVAL ? DAY))  AND aqbasket.booksellerid = ?  AND borrowers.branchcode LIKE ?  
    HAVING quantity          <> 0
        AND unitpricesupplier <> 0
        AND unitpricelib      <> 0
    
ORDER BY latesince, basketno, borrowers.branchcode, supplier


If db_scheme is pg, the $query created by trhis sub is :
SELECT aqbasket.basketno,
        aqorders.ordernumber,
        DATE(aqbasket.closedate)  AS orderdate,
        aqorders.rrp              AS unitpricesupplier,
        aqorders.ecost            AS unitpricelib,
        aqorders.claims_count     AS claims_count,
        aqorders.claimed_date     AS claimed_date,
        aqbudgets.budget_name     AS budget,
        borrowers.branchcode      AS branch,
        aqbooksellers.name        AS supplier,
        aqbooksellers.id          AS supplierid,
        biblio.author, biblio.title,
        biblioitems.publishercode AS publisher,
        biblioitems.publicationyear,
    
    aqorders.quantity - COALESCE(aqorders.quantityreceived, 0) AS quantity,
    (aqorders.quantity - COALESCE(aqorders.quantityreceived, 0)) * aqorders.rrp AS subtotal,
    (CAST(now() AS date) - closedate) AS latesince
     
    FROM
        aqorders LEFT JOIN biblio     ON biblio.biblionumber         = aqorders.biblionumber
        LEFT JOIN biblioitems         ON biblioitems.biblionumber    = biblio.biblionumber
        LEFT JOIN aqbudgets           ON aqorders.budget_id          = aqbudgets.budget_id,
        aqbasket LEFT JOIN borrowers  ON aqbasket.authorisedby       = borrowers.borrowernumber
        LEFT JOIN aqbooksellers       ON aqbasket.booksellerid       = aqbooksellers.id
        WHERE aqorders.basketno = aqbasket.basketno
        AND ( datereceived = ''
            OR datereceived IS NULL
            OR aqorders.quantityreceived < aqorders.quantity
        )
        AND (aqorders.datecancellationprinted IS NULL OR aqorders.datecancellationprinted='0000-00-00')
     AND (closedate <= CAST(now() AS date) - interval '? DAY')  AND aqbasket.booksellerid = ?  AND borrowers.branchcode LIKE ?  
    HAVING quantity          <> 0
        AND unitpricesupplier <> 0
        AND unitpricelib      <> 0
    
ORDER BY latesince, basketno, borrowers.branchcode, supplier
Comment 16 Chris Nighswonger 2012-03-23 20:21:46 UTC
IIRC, the QAM has the prerogative to put bugs "in discussion." This is one which should be discussed regardless of the "my plan works fine" declaration. Database agnosticism is bigger than just Pg. Furthermore, the last time an entity undertook to quickly push "custom" database agnostic work we got stuck with C4:SQLHelper which has its own set of problems in spite of the declaration that it worked fine.

Moving back to "In Discussion"
Comment 17 Chris Cormack 2012-03-25 07:18:45 UTC
 * For hash refs, use $myhash->{mykey}, not $$myhash{mykey}


From http://wiki.koha-community.org/wiki/Coding_Guidelines

Since we are making people resubmit patches for mysqlism, then this should be resubmitted for style violations
Comment 18 MJ Ray (software.coop) 2012-03-26 14:04:00 UTC
After http://article.gmane.org/gmane.education.libraries.koha.devel/7566 and reviewing the patch, I think the current patch also fails coding guideline SQL9 because it will result in ways to avoid the placeholder system and inject SQL, particularly in date fields.
Comment 19 Marc Balmer 2012-03-27 06:23:11 UTC
The proposed patch does not introduce new vulnerabilities or possibilities for SQL injection and you can not prevent the placeholder system.  It does a mere text replacement, so when you feed '?' to one of the (date) functions, the resulting SQL code will contain the '?' at the right place and $dbh->prepare() will do it's job like before.

The right flow of things is as follows:

$sql = $dbq->dateDiff('?', '?')
$qth = $dbh->prepare($sql)
$res = $qth->execute()
Comment 20 Ian Walls 2012-05-29 20:35:49 UTC
Bugzilla has done some great work on cross-database compatibility, and their guide can be referenced here: http://www.bugzilla.org/docs/developer.html#sql-crossdb
Comment 21 Mark Tompsett 2013-06-30 01:44:04 UTC
Well, bug 7785 is listed as blocking this. I have provided a patch which removes the MySQLism from C4::Members->column. Though, if there is a RDBMS specific LIMIT-like clause, the function will still need modifications for optimization purposes, not out of necessity.

GPML...
Comment 22 Galen Charlton 2013-12-11 16:19:13 UTC
Given the new use of DBIx::Class in Koha, I'm taking this opportunity to change the title of the bug to a positive sentence: Koha should support PostgreSQL.
Comment 23 Leandro 2014-05-29 03:08:46 UTC
Hi all,

I'm evaluating the use of Koha, but our platform uses PostgreSQL.

I read your very interesting discussion and I'm interested to know the current PostgreSQL support status.

Seems like this patch is not included in the main branch yet, right? In that case, hope Koha supports PostgreSQL officially soon.

If Koha don't support PostgreSQL yet, do you know if there are a "mantained" patch? I mean, I want know if Stephane or somebody else is updating an unofficial patch to support PostgreSQL on each new release of Koha. If so, is the patch used on  production installations?

Thanks in advance.

Best Regards,
Leandro
Comment 24 David Cook 2014-11-27 23:03:03 UTC
"InnoDB allows a foreign key constraint to reference a non-unique key. This is an InnoDB extension to standard SQL."

This is possibly also worth noting. I'm not sure how PostgeSQL handles foreign key constraints on non-unique keys, but I know we have at least two of these in kohastructure.sql at the moment.
Comment 25 Mark Hofstetter 2020-06-16 20:28:30 UTC
Hi,

I would like to use postgres/Oracle with koha, and be willing to implement it. Has anybody started this - and/or would be willing to give me hints on the structure needed

cheers 
Mark
Comment 26 David Cook 2020-06-17 02:31:07 UTC
(In reply to Mark Hofstetter from comment #25)
> Hi,
> 
> I would like to use postgres/Oracle with koha, and be willing to implement
> it. Has anybody started this - and/or would be willing to give me hints on
> the structure needed
> 
> cheers 
> Mark

I think BibLibre did work on this in the past, but I can't imagine this work ever being successful, since there are many MySQLisms in Koha.

While Koha does use an ORM in many places now, there are still queries written by hand, and the database creation scripts all use MySQL-specific syntax. Plus there is that old comment of mine about the MySQL extension to InnoDB. 

It would be easier to adapt one's own system to use MySQL than to adapt Koha to use a different database, I reckon.