Provide support for ParameterMode.REF_CURSOR - Hibernate [DATAJPA-1145]

[spring-projects-issues]

Vishnudev K opened DATAJPA-1145 and commented

I was trying to execute a oracle procedure with spring-data-jpa and hibernate

PROCEDURE MY_PROC (
    P_ID IN NUMBER,
    P_PERIOD IN VARCHAR2,
    P_LIMIT IN NUMBER,
    P_CURSOR OUT T_CURSOR);

MyEntity.java

@NamedStoredProcedureQuery(
        name = "myProc",
        procedureName = "MY_PROC",
        resultClasses = ResultEntity.class,
        parameters = {
            @StoredProcedureParameter(mode = ParameterMode.IN, type = Long.class),
            @StoredProcedureParameter(mode = ParameterMode.IN, type = String.class),
            @StoredProcedureParameter(mode = ParameterMode.IN, type = Long.class),
            @StoredProcedureParameter(mode = ParameterMode.REF_CURSOR, type = void.class)

MyRepository.java

@Procedure(name = "myProc", procedureName = "MY_PROC")
    List<ResultEntity> execMyProc(Long userId,String period,Long idClientLimit);

StoredProcedureQuery query = entityManager.createNamedStoredProcedureQuery("extractWebUser");
query.setParameter(1, userId);
query.setParameter(2, period);
query.setParameter(3, idClientLimit);
query.execute();
List resultList = query.getResultList();

But on spring-data we need to use getOutputParameterValue Method instead of getResultList

Object outputParameterValue = query.getOutputParameterValue(4);

Then I discovered that the hibernate does not support REF_CURSOR

org.hibernate.procedure.internal.AbstractParameterRegistrationImpl.java

...
...
public T extract(CallableStatement statement) {
...
else if ( mode == ParameterMode.REF_CURSOR ) {
            throw new ParameterMisuseException( "REF_CURSOR parameters should be accessed via results" );
        }
...

As per the ticket
https://jira.spring.io/browse/DATAJPA-652
its working fine on eclipse link, but broken in hibernate for quite sometime.
(I have taken the data/details from the original ticket)

---

Reference URL: #207

Attachments:

• spring-data-jpa-bug-datajpa-652.zip (10.17 kB)

[spring-projects-issues]

Jens Schauder commented

Thanks for the PR.

I'm not too fond to blindly try getResultList() on catching a PersistenceException since the exception is quite general.

If it is thrown for some other reason than the one we are trying to handle, we'll probably get another exception and thereby hide the original one. I would be nicer to detect that we are dealing with hibernate in the first place and act accordingly. We probably should add this to org.springframework.data.jpa.provider.PersistenceProvider as a new method, get the proper instance of that enum and invoke it.

Also, we should check what the spec says about this and if the behavior of Hibernate (or the others) is actually a bug

[spring-projects-issues]

Jens Schauder commented

I looked into the spec and I'd say it is a Hibernate bug. From section 3.10.17.3 Stored Procedure Query Execution of the JPA2.1 spec:

When using REF_CURSOR parameters for result sets, the update counts should be exhausted before
calling getResultList to retrieve the result set. Alternatively, the REF_CURSOR result set can be
retrieved through getOutputParameterValue. Result set mappings will be applied to results corresponding
to REF_CURSOR parameters in the order the REF_CURSOR parameters were registered
with the query.

In my interpretation, this means both getResultList and getOutputParameterValue should be supported.

Of course, this also means that we could switch to getResultList, but that does not allow for index/name as a parameter.

@kvishnudev can you please file a bug against Hibernate?

[spring-projects-issues]

Vishnudev K commented

I have raised a ticket in hibernate bucket also.https://hibernate.atlassian.net/browse/HHH-11863

But I feel this is relatively a straight forward scenario that we can fix instead of depending on Hibernate

[spring-projects-issues]

Vishnudev K commented

Hibrenate community is dead unlike spring data. :(

It does't looks like we will have a fix from hibernate anytime soon, Can we have a fix here?

[spring-projects-issues]

Jens Schauder commented

To defend the Hibernate community: We have some pretty stale issues as well ...

Anyway: You are right, doesn't look like we get a fix from Hibernate anytime soon, so I think a workaround from our side would be appropriate.

[spring-projects-issues]

Jens Schauder commented

A test case reproducing the issue would be most helpful

[spring-projects-issues]

Vishnudev K commented

I have attached a zip file to replicate this issue.
Could you see that one?

Thank you for sparing time on this

[spring-projects-issues]

Jens Schauder commented

Oh, overlooked that one. I'll take a look

[spring-projects-issues]

Jens Schauder commented

I wasn't able to reproduce the issue.

The project in the attached file tries to create an EclipeLink entity manager (and fails due to missing/wrongly configured agent).

The maven build doesn't work, because it references a snapshot version that is long gone.

I created a little project to play around with stored procedures: https://github.com/schauder/calling-oracle-stored-procedures-with-cursors
Can you fork it and create a version reproducing the issue? The readme contains instructions what you have to do to make it executable. (providing a database and adding the oracle driver)

[spring-projects-issues]

Vishnudev K commented

I could replicate that issue, I gave you a pull request for the same.
Please have a look at that one. let me know anything required

[spring-projects-issues]

Vishnudev K commented

Made changes?

[spring-projects-issues]

Jens Schauder commented

I tried implementing a workaround.

The idea is to add a
{{ if (storedProcedureQuery.hasMoreResults())
return storedProcedureQuery.getResultList();}}

before we try to access getOutputParameterValue

Unfortunately Hibernate returns false for hasMoreResults().

This makes implementing a workaround rather invasive because we would now have to lug around the information if we are expecting a ResultSet as out parameter in order to call the correct method. I don't think it makes sense under these circumstances to implement a workaround for this Hibernate bug in Spring Data

I updated the Hibernate issue with a link to my repo for demonstrating the behavior

[spring-projects-issues]

Vishnudev K commented

I could see hibernate is returning

callString.hasMoreResults()

as false.
But in eclipselink callString.hasMoreResults() is returning true

Can we use a hasMoreResults check to determine whether to use getOutputParameterValue(1) or getResultList()

[spring-projects-issues]

Jens Schauder commented

At this point, I don't want to put any more effort into this.

If you can provide a pull request that works with the repository I used for testing, I'll take a look.

[spring-projects-issues]

Vishnudev K commented

Please find the pull request for with the hibernate condition check
schauder/calling-oracle-stored-procedures-with-cursors#2

Please find the branch for eclipse link demo for the same
https://github.com/vishnudevk/calling-oracle-stored-procedures-with-cursors/tree/eclipselink

[spring-projects-issues]

Jens Schauder commented

This

if(callString.hasMoreResults()){
	//calling orignal method incase of eclispse link (Any other jpa)
	System.out.println(callString.getOutputParameterValue(1));
}else{
	//in case of hibernate use this call
	System.out.println(callString.getResultList());
}

really just works by accident. What it says is: if the is a resultset use the parameter; If there is no resultset use the resultset.

If one uses a JPA provider without the bug that Hibernate has with a SP that has an out parameter not returning a resultset but a normal value hasMoreResults will most certainly return false. This would lead to getResultList() getting executed which would fail

[spring-projects-issues]

MIhalcea Vlad commented

The Hibernate issue is fixed and tested on Oracle and PostgreSQL. It will be available for 5.2.12 onwards

[spring-projects-issues]

Jens Schauder commented

The issue does not appear any longer with the new Hibernate version

[spring-projects-issues]

Jens Schauder commented

Batch closing resolved issue without a fix version and a resolution indicating that there is nothing to release (Won't fix, Invalid ...)

[spring-projects-issues]

GabrielBB commented

I made a made a PR to make this work with or without NamedProcedures: #406

[spring-projects-issues]

Jens Schauder commented

GabrielBB This issue was closed so I don't really understand what your PR is for. If you think there is something that needs fixing, please create a new issue.

The description of the PR makes me sceptical though.
Spring Data JPA is build on JPA.
We try to stay away from database specific stuff

[spring-projects-issues]

GabrielBB commented

Jens Schauder The procedure annotation has a logic to work without Named Procedure Queries, what I did is to extend that logic to return resultsets. Right now if you want to return a cursor the only way is to put all those annotations on top of your entity. But if you called a procedure with normal output variables you don’t need the annotations, it works with a simple @procedure tag in your repository. Just like @query annotation works, you put the query right there in the annotation and can even make a native query right there, without going to your entity and declaring named queries. The thing is that feature of @procedure is missing result sets and cursors. That “AND” between result sets and cursors is important. Declaring Ref_cursors are not the only way to return results sets from procedures, take for example MySQL, right now using spring-data-JPA, as far as I know, the only way to return result sets from a procedure is to make this: @query(“ CALL procedure() “ ), because MySQL doesn’t use ref_cursor for result sets. My PR makes it work using @procedure because I did exactly what you proposed some comments above, about calling getResultList instead of getParameterValue. But instead of asking for hasMoreResults, I used the Boolean that the method “execute” returns, that Boolean tells you if the is a result set coming from the database. In conclusion, what I did is make the api a little more consistent , since when we use @procedure without cursors OR resultsets we don’t used named procedure queries, we just put the procedure name in the annotation, also when using query we don’t use named queries, so my PR finishes the work so no named queries is needed in any case, and also make @procedure work with MySQL and SQL Server, which doesn’t work right now (Postgres can also return resultsets from procedures without the need of ref cursors, those procedures don’t work with spring-data-JPA neither, but with my change they do, because to work with any database the only thing you need to do is call getResultList)

[spring-projects-issues]

GabrielBB commented

Jens Schauder I will create another Ticket, indeed. Since my PR is doing several things