spring-projects / spring-batch

Spring Batch is a framework for writing batch applications using Java and Spring
http://projects.spring.io/spring-batch/
Apache License 2.0
2.71k stars 2.35k forks source link

Add Support for Native Queries to Jpa/Hibernate readers [BATCH-1340] #2241

Closed spring-projects-issues closed 14 years ago

spring-projects-issues commented 15 years ago

Lucas Ward opened BATCH-1340 and commented

The HibernateCursorItemReader and JpaPagingItemReader currently only support HQL and JPQL query strings respectively. There are a number of scenarios where a SQL string may be required. One example I have run into recently is Oracle's support for hierarchical queries (CONNECT BY PRIOR). This feature isn't supported by Hibernate in HQL and requires a SQL query. (native query in jpa) This can be worked around by using a Jdbc reader, but if a lot of queries require this it could be problematic, especially since it would be advantageous to keep your mapping solutions consistent.


Affects: 2.0.2

Attachments:

4 votes, 5 watchers

spring-projects-issues commented 15 years ago

Dave Syer commented

As far as I know native SQL is only supported by Hibernate, and is used where JdbcTemplate and RowMapper would have been just as good if not better (there is no ORM value add for native SQL). Is there any reason you can't use straight JDBC for SQL queries? Or named queries if you really want to use Hibernate?

spring-projects-issues commented 15 years ago

Lucas Ward commented

JPA supports Native queries as well, I've used it multiple times. (there's a createNativeQuery method on EntityManager) Named queries are supported by both as well. However, we would have change the way we call them in our reader, since we just all createQuery in both JPA and Hibernate, so even if we wanted to support named queries, we would have to modify the readers.

Named query support is a good example of why we should enhance these two readers. If a project has decided as a standard that they're going to use named queries across their whole application, the only place they wouldn't be able to is with batch readers. Using the SQL readers for one or two one offs is okay, but if you have to do it everywhere, it fractures things a bit. Furthermore, you would have to write your own RowMapper for those scenarios, so if you refactor an object, you would need to know if that object was also being used in batch. It would be much easier to just allow for those in our readers. It's a simple change, although I think we would need to think through how users specified between standard/named/native queries.

spring-projects-issues commented 15 years ago

Anatoly Polinsky commented

"HibernateCursorItemReader" supports named (not parameterized / native) SQL queries already since "session.getNamedQuery( queryName )" method works well with both HQL and SQL named queries.

An example (from "spring-batch-infrastructure"): Instead of an HQL query in "Foo.hbm.xml", put SQL query:

<sql-query name="allFoos">

    <return alias="foo" class="org.springframework.batch.item.sample.Foo"/>

    SELECT  id AS {foo.id}, 
            name AS {foo.name}, 
            value AS {foo.value} 
    FROM T_FOOS

</sql-query>

"HibernateCursorItemReaderNamedQueryIntegrationTests" would like it no less than HQL query :)

That is for Hibernate. Of course it would be good to get JPA's (entityManager.createNamedQuery) integrated as well.

spring-projects-issues commented 15 years ago

Lucas Ward commented

Wow, I must have missed when the 'queryName' property was added to that reader, because it definitely wasn't in the original version. I can't say I particularly care for the method of choosing one over the other though. Deciding to use the named query if it has text, but the queryString if it doesn't smells funny to me. I was thinking of potentially using some kind of inheritance structure or other technique, strictly because I didn't want to use some kind of flag or if text existed. And as you say, the JPA version still needs the same feature, although definitely implemented in a better way.

spring-projects-issues commented 15 years ago

Dave Syer commented

StringUtils.hasText works for me (it's a pretty common idiom in components that have mutually exclusive configuration options). I guess some validation during initialization would be useful if it isn't already there.

spring-projects-issues commented 15 years ago

Anatoly Polinsky commented

In this case it actually seems that using "inheritance structure"/delegation(?) would provide a greater benefit. Let's say later we need to support not only Hibernate named/native SQL, but also Hibernate named/native/parameterized SQL, then the logic in HibernateCursorItemReader may become quite messy - since now it needs to take care of HQL, named HQL, named SQL, named-parameterized SQL.

Currently (judging by Java Docs) support for SQL named queries in "HibernateCursorItemReader" is quite accidental ( in a good way :), since the code did not even expect SQL to be thrown at it (again, this is judging from JavaDocs).

Hence, I think, it make sence to extend "HibernateCursorItemReader" or make it delegate query operations (fetching the query) to an extra component. Then (just an idea) we can take a map with parameters from XML Config / Annotations, and set it on the Query using Hibernate "Query.setProperties":

https://www.hibernate.org/hib_docs/v3/api/org/hibernate/Query.html#setProperties%28java.util.Map%29

(or set a List with parameters, Map just seems more readable from configuration perspective).

"JpaPagingItemReader" can be extended/have component to delegate to in a same way. Although we might need to encapsulate setting parameteres to "javax.persistence.Query" as a Map, so parameters can be set in a same way as in the Hibernate case.

spring-projects-issues commented 14 years ago

Anatoly Polinsky commented

maintaining backwards compatibility [since it is not a major release] had me thinking for some time...

wrote a couple of integration tests for both ItemReaders, still need to finish up component tests for individual query providers.

refer to "query-provider-visual.png" to get the visual (simple class diagram) + Java Docs.

Nice thing about JPA is that introducing native SQL support [JpaNativeQueryProvider] inherited parameterization, which means that "select * from T_FOOS where value >= :limit" with "reader.setParameterValues(Collections.<String, Object>singletonMap("limit", 3));" will work exactly the same as it does with JPQL.

For the Hibernate native SQL can be used though the named SQL within the mapping file which is already supported as per comment above [which I would think is a prefered usage in case native SQL is needed], as well as though the new HibernateNativeQueryProvider [in a patch] which will map the result to the entity for a single item.

spring-projects-issues commented 14 years ago

Anatoly Polinsky commented

was mocking around with having abstract providers implementing ItemStream interface so the QueryProvider interface does not suggest statefullness => the close method would be implemented on the abstract level (rather than dictated by provider interface) and would be called by the framework...

still thinking though, since wanted to make Query Providers independent (from ItemReader / ItemStream / etc..) so they can be used outside of Spring container if needed. However opening and closing stream automatically does look compeling.

spring-projects-issues commented 14 years ago

Dave Syer commented

If you are worried about using query providers outside Batch, is there really much value in implementing ItemStream? Presumably the state management is mandatory, so the ItemReader that wraps the query provider can call the relevant methods where necessary.

spring-projects-issues commented 14 years ago

Anatoly Polinsky commented

[ Yeap, that is how it is implemented currently where ItemReader (JPA / Hibernate) calls queryProvider.close() on doClose(). ]

Adding more tests (second patch attached), so now we have these new tests in place:

HibernateCursorItemReaderNativeQueryIntegrationTests JpaPagingItemReaderNativeQueryIntegrationTests AbstractHibernateQueryProviderIntegrationTests AbstractHibernateQueryProviderTests AbstractJpaQueryProviderIntegrationTests AbstractJpaQueryProviderTests HibernateNativeQueryProviderIntegrationTests HibernateNativeQueryProviderTests JpaNativeQueryProviderIntegrationTests JpaNativeQueryProviderTests

spring-projects-issues commented 14 years ago

Dave Syer commented

Thanks for the patches. I didn't like the stateful query providers, so I refactored those to accept a session factory / entity manager through the interface (then the caller is responsible for managing the state) - the generic QueryProvider interface also seemed gratuitous, so I split that into un-parameterized Jpa and Hibernate versions. I also added named parameter support to the HibernateCursorItemReader. Refactored tests a lot (unnecessary abstracts removed).