-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix calling stored procs on Postgres, and enable some tests #2308
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general please update copyright year in the file headers.
...nce.core/src/main/java/org/eclipse/persistence/internal/databaseaccess/DatabasePlatform.java
Show resolved
Hide resolved
...istence.jpa/src/main/java/org/eclipse/persistence/internal/jpa/StoredProcedureQueryImpl.java
Outdated
Show resolved
Hide resolved
@@ -201,7 +205,7 @@ private static boolean createSimpleStoredProcedure(EntityManagerFactory emf) { | |||
//Add more platform specific diction to support more platforms | |||
if(platform.isMySQL()) { | |||
proc.addStatement("SET out_param_one = CONCAT('One: ',in_param_one,' Two: ',in_param_two,' Three: ',in_param_three)"); | |||
} else if(platform.isOracle()) { | |||
} else if(platform.isOracle() || platform.isPostgreSQL()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It throws
[ERROR] TestStoredProcedures.setup:54->createSimpleStoredProcedure:222 » Database
Internal Exception: org.postgresql.util.PSQLException: ERROR: procedures cannot have OUT arguments
Hint: INOUT arguments are permitted.
Error Code: 0
Call: CREATE PROCEDURE simple_order_procedure (
in_param_one VARCHAR(10),
in_param_two VARCHAR(10),
in_param_three VARCHAR(10),
OUT out_param_one VARCHAR(30)) AS
.....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently PG only supports OUT params since version 14. I have 17 installed, so it worked for me. I've modified the test to use INOUT instead for PG. It works on 17, and hopefully it works on whatever version you're testing with.
Copyright year has been updated for the files I changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Second release candidate, with this PR, of EclipseLink 4.0.5 (4.0.5-RC2) is available. |
Thanks. Do you want me to make another PR for master branch? Or are you going to port it yourself? |
Please make port to master. |
Will do. Do you want me to get rid of the isJDBCExecuteCompliant() method at the same time? Or just do a direct cherry-pick? |
In master branch 5.0 it should be removed |
Fixes calling stored procedures on Postgres. Without this change, EclipseLink does
SELECT * FROM name_of_proc
which causes an exception. This change also removes theisJDBCExecuteCompliant()
method, which is not needed anymore as far as I can tell, and enables a few stored procedure tests for Postgres to prevent regressions.This PR targets the 4.0 branch but would also need to be done for 5.0.