Hibernate 4 Upgrade: Issues with classes using executeSQL

Hi All,

While I’m working on the hibernate upgrade, it seems there are lot of incompatibilities coming because of hibernate core and the hibernate search upgrade. Current work on the hibernate upgrade can be found in Hiebernate Upgrade Branch. I will be need guidance on OpenMRS hibernate and hibernate search to carry on the works with hibernate 4 upgrade.

One of major point we encounter issues with the hibernate upgrade is the classes that using executeSQL metod will no longer able to pass the database connection to the executeSQL method as it’s not possible to take it from the hibernate API. We will be need to rewrite this method.

With the hibernate search upgrade, I encounter several issues witth solr and lucene.

It’s possible shall we discuss the upgrade more depth in one of dev call?

@wyclif @darius @burke @raff @dkayiwa WDYT?

Also with the latest version of hibernate search, they removed the dependency on Apache solr and fully switched to Apache lucene. I’m not quite familiar with how hibernate search has been used within OpenMRS Core. I saw solr uses in some concept classes. So we might need to get rid of solr and use lucene.

http://in.relation.to/Bloggers/AFreshNewMajorReleaseHibernateSearch500Final

This sounds like a great collection of topics for a design call. :slight_smile:

It would be helpful to bring a list of examples of the sort of things that break, and how one can fix them.

1 Like

If you create JIRA tickets (one for each method to replace/refactor for instance) I can take some of them.

1 Like

That’s the spirit! :slight_smile:

I understood Harsha to be asking about an overall approach, e.g. is it okay to require modules to make changes to run on the next version of the platform?

If it’s just a list of changes that need someone to make them, then we’re in great shape, as people like Lluis can churn the fixes out.

We have Hibernate 4 on the 23 July Dev Forum. We could probably find time on a dev forum before then… or discuss it during a design forum.

Related issue is https://issues.openmrs.org/browse/TRUNK-4364

@harsha89, my understanding is that you identified 2 major issues, which can be seen as subtasks of TRUNK-4364.

I have assessed https://issues.openmrs.org/browse/TRUNK-4676 and added a suggested solution, which should be straightforward and is ready for work.

For https://issues.openmrs.org/browse/TRUNK-4677 we need someone to identify actual issues (list any compilation errors and/or failing tests)

Regarding dropping solr dependencies…

Until this version Hibernate Search depended on Apache Lucene for most of the work, and also on Lucene’s sister project Apache Solr to provide a richer set of analyzers. Since the Lucene project incorporated this functionality from Solr, there is no longer any need to depend on Solr artifacts.

Quote from release notes. We should be good since the features have been simply moved.

@raff Yeap you are correct, I have posted two main tickets, I will go through the other issues and and create jiras for them.

@lluismf I will create tasks for each task. Thanks for your works on this.

One of most use method is

 public static List<List<Object>> executeSQL(Connection conn, String sql, boolean selectOnly)

@raff I also went on this approach. Since it’s return type is void, we may either need variable define outside of this scope and assign the value return from the execute method. With a custom work class extending the Work class will be a way to do it. I think we will also need to check on thread safety when implement the above method with following.

    session.doWork(
        new Work() {
            public void execute(Connection connection) throws SQLException 
            { 
                doSomething(connection); 
            }
        }
    ); 

@burke, @darius It’s great we can schedule some discussion before July :slight_smile:

Thanks, Harsha

Use session.doReturningWork if you need to return anything.

For what I understand, this method is useful if you want to keep the current JDBC code that uses a Connection. What I propose is to refactor the method to avoid JDBC if possible (with native queries and query.executeUpdate).

JDBC should be used just for exceptional cases, when performance is compromised, for very long jobs to avoid creating thousands of objects in memory etc.

Llluis, I totally agree that avoiding JDBC if not absolutely necessary is the way to go. I just want us to move to Hibernate 4 with the lowest number of changes and effort. Especially module developers should have an easy way to get the old code running on the latest version as quickly as possible.

Next we should slowly progress on eliminating unnecessary JDBC queries.

I’d expect that most people are calling the AdministrationService version of this method, which is just executeSQL(String sql, boolean selectOnly)

Are there lots of raw calls to DatabaseUtil.executeSQL()?

There are 19 direct calls to DatabaseUtil.executeSQL and with the administration service executeSQL calls, it’s around 30.

15 call to executeSQL are using a Connection not coming from Hibernate, but from Liquibase. There is no need to change them I believe. Package org.openmrs.util.databasechange

Good news! The master branch is now running the latest Hibernate and Hibernate Search!

Thanks a lot to @harsha89 for taking the first step, @lluismf and @spereverziev for code contributions!

The remaining work is to identify and fix modules that stopped working after the Hibernate upgrade and continue testing the platform. I’ve started to create issues for specific modules as subtasks of TRUNK-4364. Any help with that is highly appreciated!

2 Likes

Thanks to all you guys that have made this happen

Thanks all for the make this happen. I couldn’t do much contributions after initial start. :frowning: But really happy that this has happened. :slight_smile: