Skip to content

Commit

Permalink
Fix calling stored procs on Postgres, and enable some tests (#2333)
Browse files Browse the repository at this point in the history
  • Loading branch information
@rdicroce
rdicroce authored Dec 20, 2024
1 parent e4245e2 commit 82f70b2
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 89 deletions.
10 changes: 0 additions & 10 deletions ....core/src/main/java/org/eclipse/persistence/internal/databaseaccess/DatabasePlatform.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1442,16 +1442,6 @@ public boolean isInformixOuterJoin() {
return false;
}

/**
* Returns true if this platform complies with the expected behavior from
* a jdbc execute call. Most platforms do, some have issues:
*
* @see PostgreSQLPlatform
*/
public boolean isJDBCExecuteCompliant() {
return true;
}

/**
* Return true is the given exception occurred as a result of a lock
* time out exception (WAIT clause). If sub-platform supports this clause,
Expand Down
77 changes: 10 additions & 67 deletions ...ence.core/src/main/java/org/eclipse/persistence/platform/database/PostgreSQLPlatform.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@
import org.eclipse.persistence.expressions.Expression;
import org.eclipse.persistence.expressions.ExpressionOperator;
import org.eclipse.persistence.internal.databaseaccess.DatabaseCall;
import org.eclipse.persistence.internal.databaseaccess.DatasourceCall;
import org.eclipse.persistence.internal.databaseaccess.DatasourceCall.ParameterType;
import org.eclipse.persistence.internal.databaseaccess.FieldTypeDefinition;
import org.eclipse.persistence.internal.expressions.ExpressionJavaPrinter;
import org.eclipse.persistence.internal.expressions.ExpressionSQLPrinter;
Expand All @@ -36,17 +34,14 @@
import org.eclipse.persistence.internal.helper.ClassConstants;
import org.eclipse.persistence.internal.helper.DatabaseField;
import org.eclipse.persistence.internal.helper.DatabaseTable;
import org.eclipse.persistence.internal.sessions.AbstractRecord;
import org.eclipse.persistence.internal.sessions.AbstractSession;
import org.eclipse.persistence.mappings.structures.ObjectRelationalDatabaseField;
import org.eclipse.persistence.queries.SQLCall;
import org.eclipse.persistence.queries.StoredProcedureCall;
import org.eclipse.persistence.queries.ValueReadQuery;
import org.eclipse.persistence.tools.schemaframework.FieldDefinition;

import java.io.CharArrayWriter;
import java.io.IOException;
import java.io.StringWriter;
import java.io.Writer;
import java.sql.CallableStatement;
import java.sql.PreparedStatement;
Expand Down Expand Up @@ -303,20 +298,6 @@ public boolean shouldPrintOutputTokenAtStart() {
return true;
}

/**
* Calling a stored procedure query on PostgreSQL with no output parameters
* always returns true from an execute call regardless if a result set is
* returned or not. This flag will help avoid throwing a JPA mandated
* exception on an executeUpdate call (which calls jdbc execute and checks
* the return value to ensure no results sets are returned (true))
*
* @see PostgreSQLPlatform
*/
@Override
public boolean isJDBCExecuteCompliant() {
return false;
}

/**
* INTERNAL: Answers whether platform is Postgres.
*/
Expand Down Expand Up @@ -520,63 +501,25 @@ public int getMaxFieldNameSize() {
*/
@Override
public String getProcedureBeginString() {
return "AS $$ BEGIN ";
return "$$ BEGIN ";
}

/**
* INTERNAL: Used for sp calls.
*/
@Override
public String getProcedureEndString() {
return "; END ; $$ LANGUAGE plpgsql;";
}

/**
* INTERNAL: Used for sp calls. PostGreSQL uses a different method for executing StoredProcedures than other platforms.
*/
@Override
public String buildProcedureCallString(StoredProcedureCall call, AbstractSession session, AbstractRecord row) {
StringWriter tailWriter = new StringWriter();
StringWriter writer = new StringWriter();
boolean outParameterFound = false;

tailWriter.write(call.getProcedureName());
tailWriter.write("(");

int indexFirst = call.getFirstParameterIndexForCallString();
int size = call.getParameters().size();
String nextBindString = "?";

for (int index = indexFirst; index < size; index++) {
String name = call.getProcedureArgumentNames().get(index);
Object parameter = call.getParameters().get(index);
ParameterType parameterType = call.getParameterTypes().get(index);
// If the argument is optional and null, ignore it.
if (!call.hasOptionalArguments() || !call.getOptionalArguments().contains(parameter) || (row.get(parameter) != null)) {
if (!DatasourceCall.isOutputParameterType(parameterType)) {
tailWriter.write(nextBindString);
nextBindString = ", ?";
} else {
if (outParameterFound) {
//multiple outs found
throw ValidationException.multipleOutParamsNotSupported(getClass().getSimpleName(), call.getProcedureName());
}
outParameterFound = true; //PostGreSQL uses a very different header to execute when there are out params
}
}
}
tailWriter.write(")");

if (outParameterFound) {
writer.write("{?= CALL ");
tailWriter.write("}");
} else {
writer.write("SELECT * FROM ");
}
writer.write(tailWriter.toString());
return "END; $$ LANGUAGE plpgsql;";
}

return writer.toString();
/**
* INTERNAL: Used for sp calls.
*/
@Override
public String getProcedureCallHeader() {
return "CALL ";
}

/**
* INTERNAL Used for stored function calls.
*/
Expand Down
24 changes: 17 additions & 7 deletions ...est.jse/src/it/java/org/eclipse/persistence/jpa/test/storedproc/TestStoredProcedures.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2020 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019 IBM Corporation. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand Down Expand Up @@ -30,6 +30,7 @@
import org.eclipse.persistence.jpa.test.framework.Property;
import org.eclipse.persistence.platform.database.DatabasePlatform;
import org.eclipse.persistence.sessions.DatabaseSession;
import org.eclipse.persistence.tools.schemaframework.FieldDefinition;
import org.eclipse.persistence.tools.schemaframework.SchemaManager;
import org.eclipse.persistence.tools.schemaframework.StoredProcedureDefinition;

Expand Down Expand Up @@ -121,6 +122,8 @@ public void testStoredProcedure_SetUnordered_IndexParameters() {
*/
@Test
public void testStoredProcedure_SetOrdered_NamedParameters() {
Assume.assumeFalse("pgjdbc does not support named parameters", getPlatform(storedProcedureEmf).isPostgreSQL());

EntityManager em = storedProcedureEmf.createEntityManager();
try {
StoredProcedureQuery storedProcedure = em.createStoredProcedureQuery("simple_order_procedure");
Expand Down Expand Up @@ -152,6 +155,8 @@ public void testStoredProcedure_SetOrdered_NamedParameters() {
*/
@Test
public void testStoredProcedure_SetUnordered_NamedParameters() {
Assume.assumeFalse("pgjdbc does not support named parameters", getPlatform(storedProcedureEmf).isPostgreSQL());

EntityManager em = storedProcedureEmf.createEntityManager();
try {
StoredProcedureQuery storedProcedure = em.createStoredProcedureQuery("simple_order_procedure");
Expand Down Expand Up @@ -186,22 +191,27 @@ private static boolean createSimpleStoredProcedure(EntityManagerFactory emf) {
//Setup a stored procedure
EntityManager em = emf.createEntityManager();
try {
DatabaseSession dbs = ((EntityManagerImpl)em).getDatabaseSession();
SchemaManager manager = new SchemaManager(dbs);
Platform platform = dbs.getDatasourcePlatform();

StoredProcedureDefinition proc = new StoredProcedureDefinition();
proc.setName("simple_order_procedure");

proc.addArgument("in_param_one", String.class, 10);
proc.addArgument("in_param_two", String.class, 10);
proc.addArgument("in_param_three", String.class, 10);
proc.addOutputArgument("out_param_one", String.class, 30);

DatabaseSession dbs = ((EntityManagerImpl)em).getDatabaseSession();
SchemaManager manager = new SchemaManager(dbs);
Platform platform = dbs.getDatasourcePlatform();
if (platform.isPostgreSQL()) {
// PG only supports OUT in 14+
proc.addInOutputArgument(new FieldDefinition("out_param_one", String.class, 30));
} else {
proc.addOutputArgument("out_param_one", String.class, 30);
}

//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()) {
proc.addStatement("out_param_one := 'One: ' || in_param_one || ' Two: ' || in_param_two || ' Three: ' || in_param_three");
} else if (platform.isDB2() || platform.isDB2Z()) {
proc.addStatement("SET out_param_one = 'One: ' || in_param_one || ' Two: ' || in_param_two || ' Three: ' || in_param_three");
Expand Down
6 changes: 1 addition & 5 deletions ...ence.jpa/src/main/java/org/eclipse/persistence/internal/jpa/StoredProcedureQueryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,11 +380,7 @@ public int executeUpdate() {

// If the return value is true indicating a result set then throw an exception.
if (execute()) {
if (getActiveSession().getPlatform().isJDBCExecuteCompliant()) {
throw new IllegalStateException(ExceptionLocalization.buildMessage("incorrect_spq_query_for_execute_update"));
} else {
return getUpdateCount();
}
throw new IllegalStateException(ExceptionLocalization.buildMessage("incorrect_spq_query_for_execute_update"));
} else {
return getUpdateCount();
}
Expand Down

0 comments on commit 82f70b2

Please sign in to comment.