Wrong handling of positional INOUT parameters when extracting output parameters about spring-data-jpa HOT 9 CLOSED

I'm working on it. I hope to provide one by Monday

from spring-data-jpa.

Thank you @thjanssen! You don't happen to have a test at hand that reproduces the problem?

from spring-data-jpa.

@christophstrobl I added a test case here: https://github.com/thjanssen/spring-data-jpa/blob/issue/3460/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/procedures/PostgresStoredProcedureIntegrationTests.java

It fails when I run it within my IDE, but the Maven build still passes. Any idea why?

BTW: The other test cases in that class were deactivated, but they seem to work fine. I've reactivated them on my branch.

from spring-data-jpa.

Suggested fix:
Add position information to ProcedureParameter class, set it in StoredProcedureAttributeSource.extractOutputParametersFrom(...), and use that position in StoredProcedureJpaQuery.extractOutputParameterValue to get the result value.
This should work based on the assumption that NamedStoredProcedureQuery.parameters() returns the parameters in the correct order. As far as I understand the rest of the code, it already makes this assumption when using positional parameters.

I did a quick prototype implementation of this fix. You can find it here: 5e19dba
The final version would need some cleanup, and the output format feels a little strange. But it seems to fix the issue without breaking anything else.

from spring-data-jpa.

thank you @thjanssen!

from spring-data-jpa.

Hey @christophstrobl
If you think my suggested fix is the right way to go, I would love to clean it up and provide a pull request.

Any suggestions on how to improve the result presentation?
Using the position as the map key looks a little strange in the test case, but I didn't come up with a better idea.

BTW: This change also fixes an issue in the current implementation that all positional output parameters use the same map key and override each other.

from spring-data-jpa.

not right off the top of my head. let me have a look at it (may take a bit of time).

from spring-data-jpa.

I think the Map is fine in the test. care to open a PR, we can take it from there :)

from spring-data-jpa.

@christophstrobl Awesome. I've send a PR

from spring-data-jpa.