Support For PostgreSQL

teleivo · June 9, 2015, 7:42am

Dear all!

I have seen

I would be happy to use OpenMRS with PostgreSQL and maybe others as well

So I am wondering if I could start the same process for PostgreSQL? Is it the right time, or do you think it would be easier to wait until

https://issues.openmrs.org/browse/TRUNK-4689

is done?

wyclif · June 10, 2015, 12:46am

Our long term goal is to support several DBMS that are supported by the libraries(hibernate and liquibase) we use including PostgresSQL but we chose to start with MariaDB

teleivo · June 10, 2015, 6:51am

Yes, thats what I understood from the forums and JIRA roadmap. I guess once the testing issue mentioned is resolved it will be easier to repeat the work for PostgreSQL:

Just wanted to raise my hand, so please feel free to adress me if you would like to start supporting PostgreSQL. I would love to see OpenMRS with PostgreSQL

dkayiwa · June 10, 2015, 9:08am

And by the way, if you have some time, you are completely free to immediately work on PostgreSQL support, if that is what you find most interesting to volunteer on.

darius · June 10, 2015, 4:26pm

+1 to @dkayiwa’s comment, and to @teleivo’s desire to help with this! I’ve always been in the prefers-postgres camp, and I’d love to see OpenMRS run on PostgreSQL.

There are two steps in the process:

Manually make lots of fixes to ensure the application works with $database
Add CI plans so this is constantly tested, and if this breaks, we know quickly

For MariaDB, step 1 was trivial (due to its near-equivalence to MySQL), but for PostgreSQL, step 1 will be big. It would be great if you can start looking into this.

I would suggest that the first step is to try a clean installation where in the first-time setup wizard you point it to a postgres install instead of mysql. I expect this will fail, because the liquibase changesets added since 1.9 don’t support postgres.

Once you’ve identified some of the errors, it’s likely that the fixes to these are similar to things that @sunbiz did years back. (There must be a commit against the liquibase-update-to-latest.xml file by him that mentions postgres in the commit comment.)

Then we’ll need to look at all the modules bundled in OpenMRS 2.2 and ensure they similarly work with postgres. (I expect this will be easy for most modules, but hard for the reporting module.)

Let us know how it goes!

lluismf · June 10, 2015, 7:17pm

My guess is that step 1 will be easy too, at least in core. If there were propietary SQL extensions used the H2 database (the one used running unit tests) would fail.

teleivo · June 11, 2015, 3:42pm

dear @darius so I started with deploying openmrs.war version 11.3 into tomcat 6.29 on PostgreSQL 9.3.7

As you pointed out I get errros due to liquibase-update-to-latest.xml

There are some inserts of integer values into boolean columns. For example:

There was an error while updating the database to the latest. file: liquibase-update-to-latest.xml. Error: Migration failed for change set liquibase-update-to-latest.xml::201403070132-TRUNK-4286::andras-szell: Reason: liquibase.exception.DatabaseException: Error executing SQL INSERT INTO order_type (creator, date_created, description, java_class_name, name, retired, uuid) VALUES (1, '2014-03-09', 'Order type for test orders', 'org.openmrs.TestOrder', 'Test Order', 0, '52a447d3-a64a-11e3-9aeb-50e549534c5e'): ERROR: column "retired" is of type boolean but expression is of type integer Hint: You will need to rewrite or cast the expression.

For now I guess I can just create my own branch of the openmrs-core 1.11.3 version, make the necessary changes, compile the module and deploy it until the installation runs without errors.

Should we already create a separate branch in core where I commit these changes to? Or should I first make the changes in my fork and show you the results?

lluismf · June 11, 2015, 4:07pm

Boolean was added late to the ANSI SQL and is not supported by all databases (it’s optional). Oracle for instance has no boolean type. Perhaps we can use a smallint instead?

sunbiz · June 11, 2015, 4:41pm

The fix is to actually use valueBoolean instead of valueNumeric for the retired column. Liquibase manages to convert the boolean type to the correct native types.

@teleivo, this was the related issue on which work was done to fix the installation to work on Postgres and MS SQL:

sunbiz · June 11, 2015, 4:44pm

@darius, I don’t think the current CI tests the installation process? I’m not sure if its possible to do that out-of-the-box using Bamboo. May be @cintiadr knows?

darius · June 11, 2015, 5:00pm

Currently CI does not test the installation process. I think it’s vital that we start doing that testing in CI, on all our supported DBs, as I mentioned on this ticket:

teleivo · June 11, 2015, 8:05pm

thanks @sunbiz and @lluismf, I’ll look into applying this fix

teleivo · June 16, 2015, 12:37pm

So for the installation of OpenMRS 1.11.3 on PostgreSQL 9.3.8: Installation without demo data worked fine after fixing these 2 errors

@sunbiz fix of liquibase-update-to-latest.xml <changeSet id="201403070132-TRUNK-4286" author="andras-szell"> worked fine, changing valueNumeric = “0” to valueBoolean = “false” did the trick!
error at liquibase-update-to-latest.xml <changeSet id="201403061758-TRUNK-4284" author="Banka, Vinay"> The reason is that liquibase-core-data.xml <changeSet author="ben (generated)" id="1226348923233-15"> inserts 15 rows into table concept_class. Now since these inserts set the concept_class_id explicitly the PostgreSQL sequence concept_class_concept_class_id_seq does not get incremented automatically. So liquibase-update-to-latest.xml fails because of a duplicate key constraint as this particular changeset inserts a row into concept_class without specifying the concept_class_id. PostgreSQL thus tries to insert concept_class_id=1 which already exists.

Options/Solutions - liquibase has an alterSequence but unfortunately this does not provide means to reset the sequence value - we could execute an ALTER SEQUENCE statement for dbms=“postgresql” to RESTART the sequence at 16 see http://www.postgresql.org/docs/9.3/static/sql-altersequence.html - But I favor the simplest solution (which I already tested and works): adapt the insert to explicitly specify the concept_class_id=16

Since I did the installation without the demo data, I’ll check that next!

And for running OpenMRS 1.11.3 on PostgreSQL 9.3.8:

I tried to create a new patient and got exception

ERROR - JDBCExceptionReporter.logExceptions(234) |2015-06-16 13:59:57,019| ERROR: function soundex(character varying) does not exist

this error is due to the PersonDAO.getSimilarPeople implementation which uses the function soundex which comes with MySQL by default but is not installed by default on PostgreSQL but is in the contrib modules. So I will try to install that addon and test creating a patient again.

PS: if anybody wants to quickly create a VM with tomcat 6.0.29, PostgreSQL 9.3.8 with an empty db and user ready for OpenMRS deployment, I put this together in a vagrant/virtualbox/puppet setup https://github.com/teleivo/puppet-openmrs-postgres

darius · June 16, 2015, 2:07pm

@teleivo, this is great! I love it!

Comments:

I do not think we can explicitly set a concept_class_id on <changeSet id="201403061758-TRUNK-4284" author="Banka, Vinay">. Generally speaking, since implementations may have created their own concept_classes after the initial installation, we can’t know that an update is going to be the next in the sequence. (And the same goes for any other type.) The outcome we need is that after the liquibase-core-data is run, we end up with all sequences in the right place. It could be the alter sequence approach, or else we could rewrite the insert of the 15 rows to properly pull from the sequence (or autonumber) instead of having hardcoded ids.

That code that uses soundex is very old (before May 2007). At this point, since we’ve now added Lucene/hibernate-search, I think the best approach is to actually start applying those to person and patients searches, and to rewrite the PersonDAO.getSimilarPeople method to use lucene/hibernate-search from 1.11.x onwards. (For now, though, yes, please test whether adding a PostgreSQL contrib addon is a temporary solution.)

sunbiz · June 16, 2015, 7:58pm

Using ALTER SEQUENCE statement by setting its RESTART value to 16 is going to be problematic, for the same reason that @darius mentioned below. Instead, you want to write a subquery that will do a row count and setval for the sequence to that… and then the normal insert changeset should work fine.

As for the getSimilarPeople, I’ve found that method to be unreliable and buggy. Particularly because soundex doesn’t work well for non-English names. PostgreSQL installer provided C:\Program Files\PostgreSQL\9.3\share\extension\fuzzystrmatch–1.0.sql for me and executing this script added the soundex, dmetaphone and metaphone functions.

teleivo · May 8, 2016, 10:56am

Dear all,

I know it has been a while, I got stuck with the radiology module.

This topic is still dear to my heart.

Should I create a new issue like https://issues.openmrs.org/browse/TRUNK-4689 for PostgreSQL ?
My aim is that the platform runs on PostgreSQL, good/bad idea ? (I mean I am not going to make PRs for 1.11.x, 1.12.x)
Any news regarding the CI plan ?

dkayiwa · May 8, 2016, 12:05pm

You can go ahead and create the ticket for PostgreSQL.
That would definitely be a great idea.
We disabled some CI plans because they were constantly failing. If one can work on the ticket and finish it, we can then start with a non failing CI plan.

teleivo · May 8, 2016, 3:32pm

dear @dkayiwa

created an epic https://issues.openmrs.org/browse/TRUNK-4881 with a subtask to run OpenMRS 1.11.6 on Postgresql

dkayiwa · May 8, 2016, 4:30pm

It is no longer possible with 1.11.6 because it is already released. I would make the CI task a separate ticket. It would also be helpful to include a link for this talk thread.

teleivo · May 8, 2016, 5:19pm

you are totally right, updated it to 1.11.7-SNAPSHOT. but what is the best strategy here. I mean is it better if I just use the master branch and we’ll backport it to other branches?